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

Enforce HTTP timeouts on a per-read (rather than per-request) basis #3144

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Apr 19, 2024

Summary

This leverages the new read_timeout property, which ensures that (like pip) our timeout is not applied to the entire request, but rather, to each individual read operation.

Closes: #1921.

See: #1912.

@charliermarsh charliermarsh requested a review from zanieb April 19, 2024 20:05
@charliermarsh charliermarsh marked this pull request as ready for review April 19, 2024 20:05
@charliermarsh charliermarsh added the enhancement New feature or improvement to existing functionality label Apr 19, 2024
@charliermarsh charliermarsh changed the title Update reqwest to v0.12.4 Enforce HTTP timeouts on a per-read (rather than per-request) basis Apr 19, 2024
@zanieb
Copy link
Member

zanieb commented Apr 19, 2024

I'd say this closes #1921

@charliermarsh charliermarsh merged commit 9f2bc19 into main Apr 19, 2024
38 checks passed
@charliermarsh charliermarsh deleted the charlie/req branch April 19, 2024 20:49
@notatallshaw
Copy link
Contributor

Probably fixes #2833 also

charliermarsh pushed a commit that referenced this pull request Apr 22, 2024
Since we're now using read timeouts and not total timeouts, we can use a
lower threshold, a single read shouldn't take 5 min (and not even 10s).

The 10s value is somewhat arbitrary.

Like #3144, this is a breaking change in some sense.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase or adjust scope of default HTTP timeout
3 participants