Unpin the requests library #98
Conversation
TylerLubeck
force-pushed
the
unpin_requests
branch
from
2bddfa9
to
d0806c4
Compare
|
requests 2.9.1 was an arbitrary starting point. Happy to expand this to all of 2.x, as you've demonstrated it's compatible. |
| @@ -5,6 +5,6 @@ pytest==2.7.3 | |||
| pytest-gitignore==1.3 | |||
| pytest-mock==0.9.0 | |||
| pytest-pylint==0.4.0 | |||
| requests>=2.9.1,<3 | |||
| requests | |||
There was a problem hiding this comment.
Does this promise compatibility with all versions of requests? This PR doesn't test requests 1.x. And who knows what breaking changes 3.x will incur. Makes me 😟 .
But if that's the Python way, ok!
There was a problem hiding this comment.
Unfortunately it doesn't promise compatibility. Generally, I've avoided pinning in library packages unless there's a known version of a dependency that breaks things. This allows apps to pin their dependencies as needed.
Funnily enough, this is how we found this issue in the first place. We had an internal package pinning tldextract>1.2 and an internal application pinning requests==2.2. tldextract auto-updated on a release and we ran in to a hell of version conflicts.
I just did another test run locally with requests==2.0.1 and it failed. I'll bump the pin to be >=2.1.0 since that's the lowest known working version. I'm of the opinion that leaving the upper-end unrestricted (i.e. don't pin requests<3) is alright, since it's not yet known to fail.
There was a problem hiding this comment.
Sounds good! Thanks for the explanation!
|
LGTM 👍 I am not a fan of pinning dependencies. |
|
Thanks for the quick turn around! And thanks for maintaining this package - it's been a lifesaver. |
Hi all,
Is pinning requests>2.9.1,<3 necessary? We have a few packages that rely on both tldextract and requests 2.2 (long story), and we're getting version conflicts because of it.
I ran the test suite with requests 2.2.1, requests 2.8.1, and requests 2.9.1 and they all pass.
Related to #89