fix: session object was never used in aiohttp request (#700)#701
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
@googlebot I signed it! |
804c4d3 to
dfb2440
Compare
| # TODO: Use auto_decompress property for aiohttp 3.7+ | ||
| if session is not None and session._auto_decompress: | ||
| raise ValueError( | ||
| "Client sessions with auto_decompress=True are not supported." |
There was a problem hiding this comment.
why raise the error here?
There was a problem hiding this comment.
Modifying the session object provided by the user is inappropriate in this place, so we can't disable auto_decompress ourselves. Remaining options are:
- Ignore the
sessionobject provided by the user and leave theself.session = None. Log error in console. - Raise the error to stop execution and warn user, that's something went wrong with one's
session.
I chose the latter option because it's better to fail early in my opinion. A user may not notice that one's session object is ignored.
There was a problem hiding this comment.
Sorry I meant why auto_decompress=True are not supported. I am not familiar with aiohttp.
I think in this PR you also need to pass auto_decompress=False to https://github.com/googleapis/google-auth-library-python/blob/master/system_tests/system_tests_async/conftest.py#L29 to make the system test work.
There was a problem hiding this comment.
Sorry I meant why auto_decompress=True are not supported. I am not familiar with aiohttp.
I think in this PR you also need to pass
auto_decompress=Falseto https://github.com/googleapis/google-auth-library-python/blob/master/system_tests/system_tests_async/conftest.py#L29 to make the system test work.
Ah, auto_decompress=True is supported by aiohttp, but this library prefers to do decompression on its own:
google-auth-library-python/google/auth/transport/_aiohttp_requests.py
Lines 80 to 88 in 04c870a
I've made required changes in conftest.py.
There was a problem hiding this comment.
Ah I see. Thank you! The system test is still failing. I will take a look at why it failed.
|
@arithmetic1728 I hope I found the problem. I moved the session creation process to a separate fixture. Hope it helps. |
|
@greshilov Great! Thank you! |
Fix for #700