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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix requests auto decode file #8701

Merged

Conversation

codeskyblue
Copy link
Contributor

only happens when Content-Encoding: gzip

Pull Request Check List

Resolves: #4523

only happens when Content-Encoding: gzip
@codeskyblue codeskyblue changed the title disable requests auto decode file fix requests auto decode file Nov 23, 2023
@dimbleby
Copy link
Contributor

dimbleby commented Nov 23, 2023

poetry downloads probably millions of tar.gz files every day without issue: the real problem here is that your server is misconfigured.

the proposed fix will break things for people whose servers are correctly configured and who are (admittedly unnecessarily) zipping the tar.gz for download. pip tried the same fix years ago and saw exactly that - pypa/pip#1435

Having said that, they do now seem to have landed in a place where they try to compensate for misconfigured servers by using Accept-Encoding: Identity - see all of the code in https://github.com/pypa/pip/blob/main/src/pip/_internal/network/utils.py

personally I would prefer that you corrected your server configuration and we left poetry alone, but if you want to pursue this path here then there is more to do.

edit: on reflection I think that just setting Accept-Encoding: Identity - and making no other change - should be reasonable. whl and tar.gz files are already compressed so telling servers not to compress them again during transfer is just saving compute on both sides

pyproject.toml Outdated Show resolved Hide resolved
@codeskyblue
Copy link
Contributor Author

poetry downloads probably millions of tar.gz files every day without issue: the real problem here is that your server is misconfigured.

the proposed fix will break things for people whose servers are correctly configured and who are (admittedly unnecessarily) zipping the tar.gz for download. pip tried the same fix years ago and saw exactly that - pypa/pip#1435

Having said that, they do now seem to have landed in a place where they try to compensate for misconfigured servers by using Accept-Encoding: Identity - see all of the code in https://github.com/pypa/pip/blob/main/src/pip/_internal/network/utils.py

personally I would prefer that you corrected your server configuration and we left poetry alone, but if you want to pursue this path here then there is more to do.

edit: on reflection I think that just setting Accept-Encoding: Identity - and making no other change - should be reasonable. whl and tar.gz files are already compressed so telling servers not to compress them again during transfer is just saving compute on both sides

updated code according to your request.

@dimbleby
Copy link
Contributor

looks ok to me (once you fix up the linting).

Does it work in getting your server to behave itself?

@codeskyblue
Copy link
Contributor Author

looks ok to me (once you fix up the linting).

Does it work in getting your server to behave itself?

I fix this issue by remove Content-Encoding from my server (flask based pypi mirror which is not support Accept-Encoding: Identity)

@codeskyblue
Copy link
Contributor Author

how to fix the linting? any tools?

@dimbleby
Copy link
Contributor

how to fix the linting?

Per the errors! ruff and black

pyproject.toml Outdated Show resolved Hide resolved
@codeskyblue
Copy link
Contributor Author

To make the pr simple, I removed the tests code

@Secrus
Copy link
Member

Secrus commented Dec 20, 2023

To make the pr simple, I removed the tests code

No chance for a PR to be merged without tests of any kind. If you don't want to write tests for that, close this PR.

@radoering
Copy link
Member

To make the pr simple, I removed the tests code

That just sounds so wrong. 😄

Nevertheless, I think the change itself makes sense, so I just converted your test from pytest-httpserver to httpretty, which was quite simple.

@radoering
Copy link
Member

pre-commit.ci autofix

@radoering radoering merged commit 3d7b75c into python-poetry:master Dec 20, 2023
20 checks passed
Copy link

github-actions bot commented Mar 3, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
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.

Poetry refuses to install package with correct hash
4 participants