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.

Sign up for GitHub

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

Relax requests version requirement #138

Merged
merged 1 commit into from
Jun 16, 2023
Merged

Relax requests version requirement #138

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll update the upper bound once our downstream packages update their upper bound

Copy link

@PeterJCLaw PeterJCLaw Jun 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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).

extras_require={"dev": ["pytest", "pytest-cov", "pytest-xdist", "pytest-mock",
"yapf", "pycodestyle", "autoflake", "isort", "wheel"]},
author="Serge Smertin",
Expand Down