Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Upgrade vendored requests and urllib3 #12026

Closed
wants to merge 10 commits into from

Conversation

uranusjr
Copy link
Member

@uranusjr uranusjr commented May 10, 2023

urllib3 is upgraded to 2.0.2 for thread safety.
requests is upgraded to 2.30.0 for urllib3 2.x support.
chardet is replaced by charset-normalizer.

Brotli patch to urllib3 has been re-generated since the old one does not apply cleanly anymore. urllib3 2.0 also added detection for zstandard, which I also removed since it essentially has the same issue as brotli.

Since chardet is replaced, all its usages are now replaced by charset-normalizer. This is done for pygments, which is trivial (just one line), and requests, which is much more involved. I split the previous requests patch into two to better keep track of this.

This should unblock #10480.

uranusjr added 2 commits May 10, 2023 16:19
urllib3 is upgraded to 2.0.2 for thread safety.
requests is upgraded to 2.30.0 for urllib3 2.x support.
chardet is replaced by charset-normalizer.

Brotli patch to urllib3 is removed temporarily sicne it does not apply
cleanly. It will be re-applied in subsequent commits.
This removes brotli (as previously) and zstandard (added in urllib3 2.0
in a similar fashion).
@uranusjr uranusjr marked this pull request as draft May 10, 2023 08:27
@uranusjr uranusjr force-pushed the requests-2.30-urllib3-2.0 branch from b7c0af6 to 6fa8356 Compare May 10, 2023 09:36
@uranusjr
Copy link
Member Author

uranusjr commented May 10, 2023

Urgh, psf/cachecontrol#292

The linked issue is a 🗑️🔥, tldr: cachecontrol uses strict on HTTPResponse, which has been removed in urllib3 2.0.

psf/cachecontrol#292 (comment)

It's safe to drop [strict] for urllib3 2.0 or Python 3+.

So I think I’ll just patch it out for now.

@uranusjr uranusjr force-pushed the requests-2.30-urllib3-2.0 branch from a6f9194 to 8f99623 Compare May 10, 2023 09:58
@uranusjr uranusjr force-pushed the requests-2.30-urllib3-2.0 branch from 22ada68 to 1f92c11 Compare May 10, 2023 10:13
@uranusjr
Copy link
Member Author

Apparently the network stack is completely broken now. Persumably from some thing I patched? Not sure exactly what.

@sethmlarson
Copy link
Contributor

As @nateprewitt said here, upgrading to v2.0 of urllib3 has some restrictions that pip may not want to take on yet (specifically we only support OpenSSL 1.1.1+, not LibreSSL or other flavors).

I believe the fix that spurred this change has been backported to 1.26.x so should be available to pip even if you don't upgrade to v2.0. Let me know if you have questions! :)

@Secrus

This comment was marked as outdated.

@jbylund
Copy link
Contributor

jbylund commented Jul 10, 2023

@uranusjr given the limitations re supported ssl libs do you still want to target updating urllib3 to 2.x now?

@uranusjr
Copy link
Member Author

No, I just haven’t had the time to get back to this. I still want to move to chardet-normalizer at some point though.

Also, for reference in the future, what do y’all think would be a good time to upgrade to urllib3?

@pfmoore
Copy link
Member

pfmoore commented Jul 11, 2023

Also, for reference in the future, what do y’all think would be a good time to upgrade to urllib3?

Personally, I would say whenever one of the maintainers has the time to do the upgrade (or review a submitted PR) and keep an eye on things post-merge in case of issues. But I’d say it should be done in the first half of a release cycle, so it’s settled in by the time of the release.

@jbylund
Copy link
Contributor

jbylund commented Jul 11, 2023

Not sure if you were asking in terms of when in the release cycle or when it makes sense to jump to the 2.x branch? On the 2.x branch, 2.x requires openssl 1.1.1+ , pep-644 is python 3.10+. So my possibly overly conservative leaning would be pip should stay on urllib3<2 until support for python 3.9 is dropped?

@uranusjr
Copy link
Member Author

On the 2.x branch, 2.x requires openssl 1.1.1+ , pep-644 is python 3.10+. So my possibly overly conservative leaning would be pip should stay on urllib3<2 until support for python 3.9 is dropped?

Since it’s not practical to vendor two versions of urllib3, I guess that’s the most reasonable timeline. See ya’ll in 2025!

@uranusjr uranusjr closed this Jul 12, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants