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

is_http_range_requests_unsupported should return true on Method Not Allowed #1713

Conversation

olivierlefloch
Copy link
Contributor

Summary

Azure Artifacts does not allow HEAD requests when attempting to download packages. This expands error handling in is_http_range_requests_unsupported to identify HTTP 405 (Method Not Allowed) error codes, and return true (i.e. Range requests will not be supported). This partially addresses #1458 – after this change, Azure Artifacts downloads still fail, but due to 401 Not Authorized instead of 405 Method Not Allowed.

Test Plan

I ran something akin to

RUST_LOG=trace cargo run -- pip install --index-url=https://REDACTED:[email protected]/REDACTED/_packaging/REDACTED/pypi/simple/ --upgrade --verbose private-package

without this code, and got a 405 failure:

error: Failed to download: private-package==1.2.3
  Caused by: HTTP status client error (405 Method Not Allowed) for url (https://pkgs.dev.azure.com/REDACTED/_packaging/REDACTED/pypi/download/private-package/1.2.3/private_package-1.2.3-py3-none-any.whl#sha256=REDACTED)

with this code, I get a 401 failure:

error: Failed to download: private-package==1.2.3
  Caused by: HTTP status client error (401 Unauthorized) for url (https://pkgs.dev.azure.com/REDACTED/_packaging/REDACTED/pypi/download/private-package/1.2.3/private_package-1.2.3-py3-none-any.whl#sha256=REDACTED)

Caveats

I'm not seeing a non HEAD request being reported as being fired, so I'm not sure I'm doing this correctly!

…llowed

Azure Artifacts does not allow HEAD requests when attempting to download packages. This expands error handling in `is_http_range_requests_unsupported` to identify HTTP 405 (Method Not Allowed) error codes, and return `true` (i.e. Range requests will not be supported). This partially addresses astral-sh#1458 – after this change, Azure Artifacts downloads still fail, but due to 401 Not Authorized instead of 405 Method Not Allowed.
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this does look right to me.

@charliermarsh
Copy link
Member

I suspect the 401 is related to us somehow not preserving the credentials in the URL.

@charliermarsh charliermarsh marked this pull request as ready for review February 19, 2024 20:00
@charliermarsh
Copy link
Member

@olivierlefloch - Okay from your perspective if I merge?

@olivierlefloch
Copy link
Contributor Author

Absolutely! I was unsure if this needed tests, etc., but I think it solves part of the issue!

@charliermarsh charliermarsh merged commit 220bc46 into astral-sh:main Feb 19, 2024
7 checks passed
@charliermarsh
Copy link
Member

We're still figuring out our testing infra for private registries, so it's "testing live" for now :)

@olivierlefloch olivierlefloch deleted the olivier/survive_http_405_on_head_reqs branch February 19, 2024 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants