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

Maintain compatibility between yarl and aiohttp #2514

Closed
astaff opened this issue Nov 13, 2017 · 10 comments
Closed

Maintain compatibility between yarl and aiohttp #2514

astaff opened this issue Nov 13, 2017 · 10 comments

Comments

@astaff
Copy link

astaff commented Nov 13, 2017

Long story short

After recent upgrade of yarl our build that uses aiohttp==2.2.3 broke with unexpected keyword argument 'strict' being passed from http parser. Upon investigation turned out that yarl>=0.11 is required, but a breaking change was introduced in 0.14.

Expected behaviour

One of the following:

  1. During setup install the version of yarl that maintains a contract with the version of aiohttp being installed.
  2. Raise depreciation warning when arguments are being removed in minor version update.

Actual behaviour

>= requirement combined with a breaking change introduced in minor version update resulted in aiohttp being unable to handle any request.

Steps to reproduce

Running unit tests on 2.2.3 should reproduce the issue:

Traceback (most recent call last):
  File "/Users/astaff/Development/opentrons/opentrons/.env/lib/python3.5/site-packages/aiohttp/web_protocol.py", line 278, in data_received
    messages, upgraded, tail = self._request_parser.feed_data(data)
  File "aiohttp/_http_parser.pyx", line 274, in aiohttp._http_parser.HttpParser.feed_data (aiohttp/_http_parser.c:4364)
  File "aiohttp/_http_parser.pyx", line 334, in aiohttp._http_parser.cb_on_url (aiohttp/_http_parser.c:5381)
  File "aiohttp/_http_parser.pyx", line 544, in aiohttp._http_parser._parse_url (aiohttp/_http_parser.c:8777)
  File "/Users/astaff/Development/opentrons/opentrons/.env/lib/python3.5/site-packages/aiohttp/http_writer.py", line 317, in __init__
    path = yarl.quote(path, safe='@:', protected='/', strict=False)
  File "yarl/_quoting.pyx", line 38, in yarl._quoting._quote
TypeError: _quote() got an unexpected keyword argument 'strict'

Your environment

OSX, aiohttp 2.2.3

@samuelcolvin
Copy link
Member

Makes sense.

@asvetlov I think we need a release of 2.2 branch which pins yarl<0.14, or a release of yarl which allows the strict argument but ignores it and raises depreciation error, perhaps raieses exception if strict=True.

@asvetlov
Copy link
Member

Thanks for raising the issue.
We have discussed the problem in aio-libs/yarl#123
I was pretty sure the change is backward compatible but forgot about not using yarl.URL.build() by aiohttp 2.2.
Shame on me.

@asvetlov
Copy link
Member

Hmm, maybe restoring back strict param is much easier than fixing aiohttp 2.2 autodeploy issues.

@kxepal
Copy link
Member

kxepal commented Nov 13, 2017

@asvetlov
I also think the quickest way would be to restore strict param with a new release and later fight with build issues. In the end, that parameter requires not much maintenance so it could stay there for little bit longer. Some deprecation warning would help people to abandon it quickly unless they provide reasons to keep it.

@astaff
Copy link
Author

astaff commented Nov 13, 2017

Thanks for quick response and pointing at the original discussion. And thank you for all the amazing work you are doing

@fafhrd91
Copy link
Member

@kxepal 2.2 should be fixed, otherwise it will be broken later

@asvetlov
Copy link
Member

asvetlov commented Nov 13, 2017

I hope it's done by `yarl==0.14.1'

@asvetlov
Copy link
Member

I've followed @kxepal suggestion.

@astaff please close the issue after checking.

@asvetlov
Copy link
Member

Done by a new yarl release.

@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants