Limit to major releases rather than second point. #5342
Conversation
setup.py
Outdated
| 'urllib3>=1.21.1,<1.26,!=1.25.0,!=1.25.1', | ||
| 'chardet>=3.0.2,<4', | ||
| 'idna>=2.5,<3', | ||
| 'urllib3>=1.21.1,<2,!=1.25.0,!=1.25.1', |
There was a problem hiding this comment.
I agree with the other dependencies, however probably best to leave the urllib3 version locks. Requests and urllib3 coordinate releases usually so the version locks are safe. :)
There was a problem hiding this comment.
If releases are coordinated, surely there's no need for a lockstep version pin?
There was a problem hiding this comment.
I agree with Seth that the maintainers should be managing the urllib3 version bump.
Requests digs into the internals of urllib3 in a handful of places. While urllib3 may release a non-breaking change for external APIs, those changes have broken Requests in the past. There have also been cases where a release of urllib3 introduces a non-trivial bug that requires a few weeks to fix. For that reason we usually wait a few days to a week to do the official bump.
There was a problem hiding this comment.
Okay, any reason not to just specify urllib3>=1.25,<1.26?
There was a problem hiding this comment.
The != declarations were made because the first couple releases of urllib3 1.25 had breaking changes. We'll clean those up once the next minor version is out and we bump the pin to < 1.27.
There was a problem hiding this comment.
Sure, but can you think of a reason why anyone would need, or indeed want, to stick to urllib3 1.21.1 to 1.24.x? (I ask, because as we've learned, pip's "solver" doesn't like complicated version requirements, so anything we can do to simplify, the better).
There was a problem hiding this comment.
We’ve left the wider gap open since individuals may not have complete control over their ecosystem and what version of urllib3 that’s being vended.
Eventually we’ll need to start pulling the tail up, but we haven’t really talked about when to do that yet. I’d say we’d want to support at least 3 trailing minor versions. Let’s just leave it for now.
|
Thanks for the patch, @cjw296! I'm a bit torn on this one. I agree that logistically this is a painful process for all parties involved. A minor version bump should be backwards compatible, but inevitably, that rule will be broken. The question comes down to how should a library used in a number of pieces of critical infrastructure handle that case? We've had a lot of discussion on this in #4673 (Reasoning: 1, 2). The specifics of that ticket are for urllib3, but it applies similarly to chardet and idna. The short of it being we'd rather be explicit on what we support in the event issues arise. The way pip handles conflicting dependencies makes this case more complicated, but that's unfortunately the environment we're currently in. I'm happy to have the discussion again if we've got opinions from Seth or others on the Requests team. |
|
@sethmlarson seems in agreement with what I've suggested, and I didn't realise how intertwined the |
|
chardet is pretty stable and dormant now, so I'm not so concerned about that pin. idna is kind of an unknown. It's probably safe, I'm just not excited on having to do an emergency release to pin it in the future after any potential damage has already been done. If Seth wants to approve and merge, I'm ok with it once the urllib3 pins are reverted. |
|
To be clear: I've had to do emergency releases of at least three packages as a result of this pin and the idna release. It's a risk call for you, the requests maintainers: do you want to do emergency releases of requests on every new release of a dependency, or do you want to do an emergency release under the, hopefully rare, circumstances that a dependency doesn't do semver right. |
|
@sethmlarson - over to you :-) |
The idna is in the install_requires already, so it makes sense to remove it from extra requirements. Also, poetry has been confused by this duplicated requirement. python-poetry/poetry#1449
|
Just to add weight to @cjw296 I have also had to take emergency action as a result of the idna release and requests pinning it to a minor version. From my perspective the risk of a minor release being breaking is valid but hopefully low. Though I completely acknowledge that it must be frustrating for the requests maintainers to have to resolve it with an emergency release when it inevitably happens. However debugging the issues caused today by the idna release, because requests has pinned a minor version, has been unpleasant. One specific example is that a package I maintain does not depend on requests, but a dependency does. Another dependency has idna as a dependency but is pinned on major versions. It seems pip was unable to resolve this dependency graph correctly and pulled in 2.9, breaking requests, the dependency of my package and therefore the whole package. Adding requests as a dependency to my package and pinning to This has been frustrating to untangle and has fallen on those that depend on requests to resolve. |
There was a problem hiding this comment.
This looks good to me, @nateprewitt are you going to do a release of this post-merge?
More info: psf/requests#5342 fixes #6169
More info: psf/requests#5342 [noissue]
|
Yep, I'll work on getting a cleaned up release out today or tomorrow. |
…lopment.txt related PRs to address this conflict in later versions * moto: getmoto/moto#2807 * requests: psf/requests#5342
ports r552091 updated idna to 2.10, breaking www/py-requests, and its dependents, with: raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'idna<2.9,>=2.5' distribution was not found and is required by requests Backport an upstream commit [1] that kinda addresses "most" of the root cause problem of this for future dependency updates, by not pinning on minor versions. [1] psf/requests#5342 PR: 245938 Reported by: lbartoletti, ohauer MFH: 2020Q3 X-MFH-With: 552091 git-svn-id: svn+ssh://svn.freebsd.org/ports/head@552095 35697150-7ecd-e111-bb59-0022644237b5
ports r552091 updated idna to 2.10, breaking www/py-requests, and its dependents, with: raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'idna<2.9,>=2.5' distribution was not found and is required by requests Backport an upstream commit [1] that kinda addresses "most" of the root cause problem of this for future dependency updates, by not pinning on minor versions. [1] psf/requests#5342 PR: 245938 Reported by: lbartoletti, ohauer MFH: 2020Q3 X-MFH-With: 552091
ports r552091 updated idna to 2.10, breaking www/py-requests, and its dependents, with: raise DistributionNotFound(req, requirers) pkg_resources.DistributionNotFound: The 'idna<2.9,>=2.5' distribution was not found and is required by requests Backport an upstream commit [1] that kinda addresses "most" of the root cause problem of this for future dependency updates, by not pinning on minor versions. [1] psf/requests#5342 PR: 245938 Reported by: lbartoletti, ohauer MFH: 2020Q3 X-MFH-With: 552091 git-svn-id: svn+ssh://svn.freebsd.org/ports/head@552095 35697150-7ecd-e111-bb59-0022644237b5
requests should trust dependent packages to do semver rather than artificially limiting version compatibility, which causes problems for pip.
Fixes #5341, #5337 and supercedes #5226.