-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
Remove vendored charset_normalizer #12703
Conversation
e10d5d4
to
cc114fa
Compare
@nateprewitt Would it be possible for requests to have a patch release with just this change, forked off of the most recent requests release tag? |
@pradyunsg I can take a look at backporting it if it'll be helpful. The diff here will only change 6 lines once Requests releases (lines 197-202 are the only non-pip specific changes). |
My main concern is the first line from our vendoring policy:
This doesn't really violate that but it's more in a gray area -- if it's not too much effort to cut a requests release, that would be preferable since holding this patch on our end feels a bit icky to me. 😅 That said, if it's a bunch of busywork for you, I'd be happy with just holding this patch on our end for this. :) |
Totally, I completely understand that. Let me take a look once PyCon ends and I can probably get something out early next week if that's a workable timeline. |
You're at PyCon US? We should meet! Ping me over urllib3 discord? We can figure out the logistics there, so it we don't notify various people who aren't around. :) |
Yep! Should be there this evening. I'm in the discord so happy to chat there, same handle. Looking forward to it! |
229a7f1
to
9f46361
Compare
9f46361
to
516f0b2
Compare
Alright, we released Requests 2.32.0 today which contains the change for removing chardet/charset_normalizer. I tried to keep this PR partitioned between upgrading Requests and then removing the charset_normalizer package. I think this should be good to go but it looks like testing CI is having issues on multiple PRs. I'll check back later to see if that's resolved, otherwise feel free to ping me if you have concerns in the meantime. |
Looks like checks are passing with the latest commits on |
This PR proposes removing
charset_normalizer
frompip
's vendored dependencies following a discussion in #12638 (comment). Requests provides support for charset detection on bytes payloads for some APIs, previously throughchardet
, and now throughcharset_normalizer
.pip
however doesn't utilize any of this functionality.We've merged psf/requests#6702 which makes both of these dependencies optional, allowing them to be removed when Requests is repackaged or vendored.
We likely won't release Requests 2.32.0 until some time in June, but the diff here is relatively small that overlaps with existing patches so I'm raising this now. If we'd like to wait till the official release, I'll update this after that's completed.Requests 2.32.0 was released today so this patch now includes the Requests 2.32.0 upgrade as well to facilitate this change.