Relax requests version requirement
#138
Conversation
| @@ -10,7 +10,7 @@ | |||
| version=version_data['__version__'], | |||
| packages=find_packages(exclude=["tests", "*tests.*", "*tests"]), | |||
| python_requires=">=3.7", | |||
| install_requires=["requests>=2.28.1,<2.29.0"], | |||
| install_requires=["requests>=2.28.1,<3"], | |||
There was a problem hiding this comment.
we cannot increase the upper bound, as some of our downstream packages didn't migrate yet to requests 2.29.0+, lower bound can be decreased if it passes the tests.
see 1ea8f3b and psf/requests#6432
There was a problem hiding this comment.
we cannot increase the upper bound, as some of our downstream packages didn't migrate yet to requests 2.29.0+
Surely, the downstream packages should be responsible for pinning requests?! This is a publicly available package which can't be used with a version of requests without public security vulnerabilities (https://github.com/psf/requests/blob/main/HISTORY.md#2310-2023-05-22)...
There was a problem hiding this comment.
we'll update the upper bound once our downstream packages update their upper bound
There was a problem hiding this comment.
@nfx am I correct in understanding that the issue with supporting requests 2.29+ is to do with a mismatch in available ssl modules and urllib3 version 2.x (i.e: similar to urllib3/urllib3#2168)? If so, perhaps this package could pin to urllib3<2 (as the release notes for requests 2.30 suggest) as that would appear to avoid the issue? That would allow for requests to move forward to a version which has the security fix (i.e: 2.31) while keeping to the known-working version of urllib3.
I agree with @judahrand that such pinning really ought to be the responsibility of the downstream libraries/applications which actually depend on the particular versions of requests/urllib3, however if it's important to you that this package not admit urllib3 related issues then perhaps my suggestion above presents a way forwards.
There was a problem hiding this comment.
@PeterJCLaw @judahrand Sorry for the delay in addressing this ticket. As we saw during the original bump of urllib3, major version increases with backwards incompatibility has major risks for dependent software. We wanted to verify that at least our internal products wouldn't have yet another regression if we were to unpin this too soon. Especially with the release of 2.0.3, users who do not pin urllib3 in their setup.py or requirements.txt are likely to get a version that is compatible with their system's version of SSL. So, we're ready to merge this patch. This will be released in the next minor version (0.2.0).
| @@ -10,7 +10,7 @@ | |||
| version=version_data['__version__'], | |||
| packages=find_packages(exclude=["tests", "*tests.*", "*tests"]), | |||
| python_requires=">=3.7", | |||
| install_requires=["requests>=2.28.1,<2.29.0"], | |||
| install_requires=["requests>=2.28.1,<3"], | |||
There was a problem hiding this comment.
@PeterJCLaw @judahrand Sorry for the delay in addressing this ticket. As we saw during the original bump of urllib3, major version increases with backwards incompatibility has major risks for dependent software. We wanted to verify that at least our internal products wouldn't have yet another regression if we were to unpin this too soon. Especially with the release of 2.0.3, users who do not pin urllib3 in their setup.py or requirements.txt are likely to get a version that is compatible with their system's version of SSL. So, we're ready to merge this patch. This will be released in the next minor version (0.2.0).
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #138 +/- ##
=======================================
Coverage 53.30% 53.30%
=======================================
Files 30 30
Lines 18346 18346
=======================================
Hits 9779 9779
Misses 8567 8567 ☔ View full report in Codecov by Sentry. |
mgyucht
dismissed
nfx’s stale review
We have synced offline and agreed on this approach. nfx is on holiday.
|
@judahrand can you please setup a GPG key and rebase to sign your commits? See https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification#gpg-commit-signature-verification for instructions on setting this up. |
Head branch was pushed to by a user without write access
* Added Sphinx documentation ([#184](#184), [#191](#191), [#183](#183), [#193](#193)). * Integrated with ReadTheDocs service ([#188](#188), [#189](#189), [#190](#190)). * Create a deepcopy of config in api client ([#172](#172)). * Fix client/secret auth ([#186](#186)). * Increase DBFS copy buffer size ([#185](#185)). * Move classes to other repository ([#192](#192)). * Relax `requests` version upper bound to <3 ([#138](#138)).
Changes
Closes #136
Tests
make testrun locallymake fmtapplied