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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Make Brotli support enabled by default, if appropriate urllib3 version is available #5554

Closed
wants to merge 1 commit into from

Conversation

gdubicki
Copy link
Contributor

@gdubicki gdubicki commented Aug 9, 2020

This is still a work in progress - I have to make sure that the tests are actually running, not skipped every time 馃槄 but I would like to get feedback if the direction is right.

I had to update some deps to make the tests pass after a lot of packages being updated after I added brotli to dev-packages. But I assume that this should be done anyway. Especially as there were discrepancies in the versions - f.e. pytest version in setup.py commit message declared v. 4 compatibility while the version in Pipenv was set to be at most v. 3.10.1.

Speaking of dependencies I have a doubt how to declare required urllib3 version. Technically the first with brotli support is 1.25.1, but this is a version not supported by Requests... Should I update all the references to require urllib3 >= 1.25.2 then instead of >= 1.25.1?

@gdubicki gdubicki changed the title WIP: Brotli support working WIP: Add Brotli support Aug 9, 2020
docs/community/faq.rst Outdated Show resolved Hide resolved
by the urllib3 version (>= 1.25.1) and brotli package being
present
@gdubicki
Copy link
Contributor Author

Ping! :)

@sethmlarson
Copy link
Member

FYI Requests already supports Brotli via urllib3:

import requests

http = requests.Session()
http.headers = {"Accept-Encoding": "br"}

resp = http.request("GET", "https://cloudflare.com")
print(resp.text)
print("Content-Encoding:", resp.headers["Content-Encoding"])

@gdubicki gdubicki changed the title WIP: Add Brotli support WIP: Make Brotli support enabled by default, if appropriate urllib3 version is available Oct 13, 2020
@gdubicki
Copy link
Contributor Author

@sethmlarson : yes, I know. Please see the updated PR title. I hope my intent is more clear now. :)

@dilyanpalauzov
Copy link
Contributor

dilyanpalauzov commented Mar 29, 2021

At #5783 I took a different approach: rely on urllib3 to determine whether to include ,br in the Accept-Encoding header.

@gdubicki
Copy link
Contributor Author

gdubicki commented Apr 2, 2021

Closing in favor of #5783.

@gdubicki gdubicki closed this Apr 2, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants