Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TODO: undo pin of 'aiohttp' once 'aioresponses' releases a fix #632

Closed
davidwtbuxton opened this issue Oct 24, 2020 · 5 comments · Fixed by #640
Closed

TODO: undo pin of 'aiohttp' once 'aioresponses' releases a fix #632

davidwtbuxton opened this issue Oct 24, 2020 · 5 comments · Fixed by #640
Assignees
Labels
type: process A process-related concern. May include testing, release, or the like.

Comments

@davidwtbuxton
Copy link
Contributor

Environment details

  • OS: $ sw_vers
    ProductName: Mac OS X
    ProductVersion: 10.14.6
    BuildVersion: 18G6020

  • Python version: 3.6, 3.7, 3.8

  • pip version: pip 20.2.4

  • google-auth version: 5906c85

Steps to reproduce

  1. nox -s unit

There are 9 tests that fail, all with the same error:

TypeError: __init__() missing 1 required positional argument: 'limit'

====================================================== short test summary info =======================================================
FAILED tests_async/transport/test_aiohttp_requests.py::TestCombinedResponse::test_content_compressed - TypeError: __init__() missin...
FAILED tests_async/transport/test_aiohttp_requests.py::TestResponse::test_headers_prop - TypeError: __init__() missing 1 required p...
FAILED tests_async/transport/test_aiohttp_requests.py::TestResponse::test_status_prop - TypeError: __init__() missing 1 required po...
FAILED tests_async/transport/test_aiohttp_requests.py::TestAuthorizedSession::test_request - TypeError: __init__() missing 1 requir...
FAILED tests_async/transport/test_aiohttp_requests.py::TestAuthorizedSession::test_ctx - TypeError: __init__() missing 1 required p...
FAILED tests_async/transport/test_aiohttp_requests.py::TestAuthorizedSession::test_http_headers - TypeError: __init__() missing 1 r...
FAILED tests_async/transport/test_aiohttp_requests.py::TestAuthorizedSession::test_regexp_example - TypeError: __init__() missing 1...
FAILED tests_async/transport/test_aiohttp_requests.py::TestAuthorizedSession::test_request_no_refresh - TypeError: __init__() missi...
FAILED tests_async/transport/test_aiohttp_requests.py::TestAuthorizedSession::test_request_refresh - TypeError: __init__() missing ...
============================================ 9 failed, 609 passed, 12 warnings in 33.41s =============================================

Here is the traceback for one of the failing tests:

____________________________________________ TestCombinedResponse.test_content_compressed ____________________________________________

self = <tests_async.transport.test_aiohttp_requests.TestCombinedResponse object at 0x108803160>
urllib3_mock = <function decompress at 0x10880a820>

    @mock.patch(
        "google.auth.transport._aiohttp_requests.urllib3.response.MultiDecoder.decompress",
        return_value="decompressed",
        autospec=True,
    )
    @pytest.mark.asyncio
    async def test_content_compressed(self, urllib3_mock):
        rm = core.RequestMatch(
            "url", headers={"Content-Encoding": "gzip"}, payload="compressed"
        )
>       response = await rm.build_response(core.URL("url"))

tests_async/transport/test_aiohttp_requests.py:72: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../../.virtualenv/google-auth-library-python/lib/python3.8/site-packages/aioresponses/core.py:192: in build_response
    resp = self._build_response(
../../../.virtualenv/google-auth-library-python/lib/python3.8/site-packages/aioresponses/core.py:173: in _build_response
    resp.content = stream_reader_factory(loop)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

loop = <Mock id='4437587472'>

    def stream_reader_factory(  # noqa
        loop: 'Optional[asyncio.AbstractEventLoop]' = None
    ):
        protocol = ResponseHandler(loop=loop)
>       return StreamReader(protocol, loop=loop)
E       TypeError: __init__() missing 1 required positional argument: 'limit'

../../../.virtualenv/google-auth-library-python/lib/python3.8/site-packages/aioresponses/compat.py:48: TypeError
========================================================== warnings summary ==========================================================

The root cause is a change in aiohttp version 3.7.0 which was released a few hours ago. The signature for StreamReader has changed, making the optional argument limit a required argument.

https://github.com/aio-libs/aiohttp/blob/56e78836aa7c67292ace9e256711699d51d57285/aiohttp/streams.py#L106

This change breaks aioresponses:

https://github.com/pnuckowski/aioresponses/blob/e61977f42a0164e0c572031dfb18ae95ba198df0/aioresponses/compat.py#L44

@davidwtbuxton
Copy link
Contributor Author

Potential fixes:

  • Wait for aiohttp to undo the change, making the limit argument optional again.
  • Wait for aioresponses to update so it is compatible with the changes in aiohttp.
  • Pin the version of aiohttp used when running tests <3.7.0.

@davidwtbuxton
Copy link
Contributor Author

I've made a PR for aioresponses pnuckowski/aioresponses#174

@tseaver
Copy link
Contributor

tseaver commented Oct 26, 2020

@davidwtbuxton Thanks for following up. I just added a comment to the aiohttp PR which made this breaking change.

@tseaver
Copy link
Contributor

tseaver commented Oct 26, 2020

@davidwtbuxton see #634

@busunkim96 busunkim96 added type: process A process-related concern. May include testing, release, or the like. and removed triage me I really want to be triaged. labels Oct 26, 2020
@tseaver
Copy link
Contributor

tseaver commented Oct 28, 2020

PR #634 is merged. Perhaps we should leave this issue open with a change of title to indicate that we revisit the pin once aioresponses comes up with a release fixing the issue (the aiohttp maintainer says he doesn't consider his change breaking, as users aren't supposed to instantiate StreamResponse directly).

@tseaver tseaver changed the title Tests for aiohttp_requests broken in master, TypeError exception TODO: undo pin of 'aiohttp' once 'aioresponses' releases a fix Oct 28, 2020
davidwtbuxton added a commit to davidwtbuxton/google-auth-library-python that referenced this issue Oct 29, 2020
This reverts commit 05f9524.

The compatibility bug was fixed in the aioresponses package version 0.7.1.
gcf-merge-on-green bot pushed a commit that referenced this issue Oct 29, 2020
This reverts commit 05f9524.

The compatibility bug was fixed in the aioresponses package version 0.7.1 - https://pypi.org/project/aioresponses/

Fixes #632
gcf-merge-on-green bot pushed a commit that referenced this issue Oct 29, 2020
🤖 I have created a release \*beep\* \*boop\* 
---
## [1.23.0](https://www.github.com/googleapis/google-auth-library-python/compare/v1.22.1...v1.23.0) (2020-10-29)


### Features

* Add custom scopes for access tokens from the metadata service ([#633](https://www.github.com/googleapis/google-auth-library-python/issues/633)) ([0323cf3](https://www.github.com/googleapis/google-auth-library-python/commit/0323cf390b16e8483660ac88775e8ea4e7f7702d))


### Bug Fixes

* **deps:** Revert "fix: pin 'aoihttp < 3.7.0dev' ([#634](https://www.github.com/googleapis/google-auth-library-python/issues/634))" ([#632](https://www.github.com/googleapis/google-auth-library-python/issues/632)) ([#640](https://www.github.com/googleapis/google-auth-library-python/issues/640)) ([b790e65](https://www.github.com/googleapis/google-auth-library-python/commit/b790e6535cc37591b23866027a426cde312e07c1))
* pin 'aoihttp < 3.7.0dev' ([#634](https://www.github.com/googleapis/google-auth-library-python/issues/634)) ([05f9524](https://www.github.com/googleapis/google-auth-library-python/commit/05f95246fab928fe2f445781117eeac8088497fb))
* remove checks for ancient versions of Cryptography ([#596](https://www.github.com/googleapis/google-auth-library-python/issues/596)) ([6407258](https://www.github.com/googleapis/google-auth-library-python/commit/6407258956ec42e3b722418cb7f366e5ae9272ec)), closes [/github.com//issues/595#issuecomment-683903062](https://www.github.com/googleapis//github.com/googleapis/google-auth-library-python/issues/595/issues/issuecomment-683903062)
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: process A process-related concern. May include testing, release, or the like.
Projects
None yet
4 participants