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

Add default timeout #6709

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add default timeout #6709

wants to merge 2 commits into from

Conversation

sigmavirus24
Copy link
Contributor

This adds a default connect and read timeout value for all usage of Requests. This is to solve a long-standing issue where some systems do not have a sufficiently low default value.

Personally, I'd want these values to be much lower, but a 10 second connection timeout and a 30 second read timeout seem like they should be enough to avoid problems for the edge cases of users while also not being so large that they're basically ineffective.

Closes #3070

This adds a default connect and read timeout value for all usage of
Requests. This is to solve a long-standing issue where some systems do
not have a sufficiently low default value.

Personally, I'd want these values to be much lower, but a 10 second
connection timeout and a 30 second read timeout seem like they should be
enough to avoid problems for the edge cases of users while also not
being so large that they're basically ineffective.

Closes psf#3070
@sigmavirus24 sigmavirus24 added this to the 2.33.0 milestone May 21, 2024
@cmyers009
Copy link

If you add a default timeout it may break existing code bases.

For example, if I am using a GET request to download a 1 GB file then I would have to adjust timeout to make my code compatible with this proposed change.

@sigmavirus24
Copy link
Contributor Author

If you add a default timeout it may break existing code bases.

For example, if I am using a GET request to download a 1 GB file then I would have to adjust timeout to make my code compatible with this proposed change.

You don't understand timeouts in Python. Please go read any number of past issues and blog posts about this issue before spreading FUD.

@neochine

This comment has been minimized.

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.

Consider making Timeout option required or have a default
3 participants