-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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 timeouts to all requests #5881
Conversation
Should we make this configurable, like pip does for instance? I would also suggest to define the default timeout value in a constant to avoid hard coding the value everywhere. |
I'm probably against making this configurable, just another confusing thing for users to worry about. Not sure I can be bothered about making all these value use a constant either. I don't know that there's any particularly compelling reason to think that we'd always want them all to have the same value anyway. Hmm, maybe. I could see a case for picking a bigger number than 5 though. 5 seconds of inactivity is certainly a slow server, but perhaps not slow enough to completely give up on. Anyone want to propose a number? Perhaps pip's default of 15 is a sensible lead to follow. |
I would not be opposed to configuring this through an environment variable, though given that our existing config machinery reads from |
I'll make it a constant and if y'all are motivated to make that configurable later then at least it'll only be one place to change. |
Also make it 15s rather than 5s (matching the default in pip)
I don't think this would confuse users, since the idea would be that most users won't even need to worry about configuring the timeout (since we have a sane default), but they would still have a knob to turn if they ever need to. Case in point, I almost never use the |
I won't be adding code to make this configurable. Feel free to do as you see fit with this MR: merge it and add to it, or merge it and leave it alone, or reject it. |
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. |
https://requests.readthedocs.io/en/latest/user/advanced/#timeouts
Seems like not a great default.
I've added a timeout to all the requests that I could find. Probably fixes some of the examples of poetry being "slow" or "stuck" that have been reported eg #3352, though it's hard to be sure.