fix auto decoding of gzip for s3 vhost proxied requests #8148
+86
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thrau
approved these changes
Apr 17, 2023
Thanks! Yes I am a bit wary when modifying a central piece like this one, and thought we could try it on S3 first. I guess it's also a nice part in our new push towards streaming responses, and it's good to see it seems to work this way! I'll merge after |
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As reported in #8139, the new S3 provider will decode gzip encoded requests before returning the data.
This was due to our Proxy and more specifically our
SimpleHttpClient, which usesrequestsunder the hood. Requests automatically decode the response, and we need the raw data underneath. I've duplicated our http client to not do a drastic change and only use it in the virtual host part of S3, we can then see if it creates issues or not 😄I didn't add unit tests as I believe it could be added on top of #8104, and don't want to create too many conflicts.
About requests and decoding gzip:
https://docs.python-requests.org/en/latest/community/faq/#encoded-data
fixes #8139