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

Patch update to older branches (2.0, 3.6, etc), to address yarl API-breaking minor update #4987

Closed
besfahbod opened this issue Oct 2, 2020 · 11 comments
Labels

Comments

@besfahbod
Copy link

🐞 Describe the bug

With yarl 1.6.0 breaking relied-on behavior of URL.build() API, we now have a problem where pip install aiohttp would receive a broken package configuratoin: aiohttp 3.6.2 with yarl 1.6.0.

To address this issue, I think we need a Patch update to the 3.6 branch, like 3.6.3.

Related/Blocking issues are:

@besfahbod besfahbod added the bug label Oct 2, 2020
@besfahbod besfahbod changed the title Patch update to 3.6 branch, to address yarl API-breaking minor update Patch update to older branches (2.0, 3.6, etc), to address yarl API-breaking minor update Oct 2, 2020
@webknjaz
Copy link
Member

webknjaz commented Oct 2, 2020

Once #4973 is in master, it'll need backports to branches 3.6 and 3.7. And after that, it'll probably be possible to cut a release but no promises.

hub-cap added a commit to hub-cap/rally that referenced this issue Oct 13, 2020
There is currently an issue with incompatible versions in our dependency
of aiohttp, which is causing installations to fail. Using the stricter
pip 2020 resolver, which is off by default in pip 20, fixes this
issue. The new resolver, which will be the new default in future pip
versions, is not ready for all use cases, but works fine for our use
case, so we are choosing to use it.

Relates https://discuss.python.org/t/announcement-pip-20-2-release/4863
Relates aio-libs/aiohttp@2d36ef6
Relates aio-libs/aiohttp#4987
@sethmlarson
Copy link

sethmlarson commented Oct 13, 2020

Just an FYI the Elasticsearch async implementation got hit by this. We depend on aiohttp and yarl in the [async] extra and Pip's old resolver knocks out the upper pin on yarl from aiohttp. Technically a pip bug (thank goodness the new resolver is coming) but still a thing to be aware of.

My plan is to remove yarl from the [async] extra and accept whatever yarl version aiohttp gives us and fall back on passing an unbuilt string as a last resort.

Also what versioning scheme is yarl? I didn't expect breaking changes in a minor.

@TBBle
Copy link
Contributor

TBBle commented Oct 14, 2020

So the context isn't lost, a 3.6.3 release has been cut, but it is just a quick patch on top of the 3.6.2 release to lock the yarl version to >=1.0,<1.6.0, it didn't include any of the bugfixes from the 3.6 branch since 3.6.2.

So for 3.6 series, it would be more-consistent with the 3.6.3 release to just cherry-pick that patch (bbceca7) to the 3.6 branch (but not the disable-CI change (1940503), since 3.6 already has fixes for the CI pipeline). However, if I understand @sethmlarson's comment, that patch doesn't work if other things depend on yarl directly, and a newer version of yarl ends up being used anyway.

Is there an existing PIP bug to track this problem, BTW? It seems like a nasty bug that pip can ignore version requirements like that.

@besfahbod
Copy link
Author

besfahbod commented Oct 14, 2020

Uh, that's a good point, @TBBle, that after 3.6.3 being released with yarl<1.6.0, having 3.6.4 with yarl>=1.6.0 would be a breaking change for anyone using aiohttp~3 or aiohttp~3.6 with already a yarl workaround in place.

I don't think these kinds of conflicts are something pip would consider a bug, but more of unsupported configuration. As far as I know, pip would try to get latest version of aiohttp, and try to get its dependencies work out with the rest of the config; and it would never hold back a version of aiohttp if using the latest would result in a dep version conflict. (I'm not a pip expert, so could be wrong about this.)

Probably aiohttp should swallow the pill here and hold yarl on <1.6.0 for the rest of 3.6 branch, to not break any configuration that's already adjusted in the past 2 weeks.

Wdyt?

@TBBle
Copy link
Contributor

TBBle commented Oct 14, 2020

I think that might be for-the-best, yeah. It's not ideal, but 3.6.3 is out there now, and my team had also locally added a yarl<1.6.0 to the requirements.txt where we pulled in aiohttp, so it does save us another round of changes on our projects.

I can appreciate the yarl position that this was a long-standing bug (encoded=False should have been treating % as needing encoding to %25, like any character), but they (and us) have been bitten by Hyrum's Law:

With a sufficient number of users of an API,
it does not matter what you promise in the contract:
all observable behaviors of your system
will be depended on by somebody.

@webknjaz
Copy link
Member

@TBBle it's best to maintain the exact env pins using something like pip-tools in order to avoid surprises. They can be autoupdated with things like dependabot.

@sethmlarson
Copy link

sethmlarson commented Oct 14, 2020

@webknjaz I agree that non-package deployments of Python should be using a proper resolver+lockfiles but I think users would be confused if installing a single package somehow fails to deliver proper dependencies. ie pip install elasticsearch[async]

danielmitterdorfer pushed a commit to elastic/rally that referenced this issue Oct 15, 2020
There is currently an issue with incompatible versions in our dependency
of aiohttp, which is causing installations to fail. Using the stricter
pip 2020 resolver, which is off by default in pip 20, fixes this
issue. The new resolver, which will be the new default in future pip
versions, is not ready for all use cases, but works fine for our use
case, so we are choosing to use it.

Relates https://discuss.python.org/t/announcement-pip-20-2-release/4863
Relates aio-libs/aiohttp@2d36ef6
Relates aio-libs/aiohttp#4987
@asvetlov
Copy link
Member

3.6.3 is released with yarl pinning.
Sorry, a patch for 2.0 and others is problematic because the build bot for old branches is not used anymore and broken. We can build sourcecode-only tarball in the best case.

@besfahbod
Copy link
Author

Thanks, @asvetlov, for the info. Apparently 2.0 already has yarl<1.11.0 (or something like that) in place, so doesn't need to be patched.

Now, with 3.6.3 being out, and the possible future of not updating yarl for this branch, I guess there's nothing else to do here. What do you think?

@asvetlov
Copy link
Member

There is #4973 which makes aiohttp compatible with the newest yarl.
I need to review it carefully.
My plan is publishing aiohttp 3.7 with this PR

@asvetlov
Copy link
Member

3.6 line (3.6.3) has pinned dependency.
I think the problem is solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants