Conversation
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).
konstin
added
the
enhancement
|
I think we should probably use a larger value given all the problems we've seen with timeouts? |
|
I guess I'd still prefer something like 30s, agree that 5 minutes is execessive. |
| // Timeout options, matching https://doc.rust-lang.org/nightly/cargo/reference/config.html#httptimeout | ||
| // `UV_REQUEST_TIMEOUT` is provided for backwards compatibility with v0.1.6 | ||
| let default_timeout = 5 * 60; | ||
| let default_timeout = 30; |
There was a problem hiding this comment.
Looks like the 5 min were previously undocumented.
It's also a bit misleading since it's not an http timeout properly, it's just a read timeout.
There was a problem hiding this comment.
👍 I don't think we'll ever want a real timeout per http request, I think a read timeout makes sense. If we expose a comprehensive timeout, I think it'd expect it to be like for an entire command invocation or installation of a package or something.
There was a problem hiding this comment.
For what it's worth,
Poetry and Pip use 15 seconds as a default
- https://python-poetry.org/docs/faq/#my-requests-are-timing-out
- https://github.com/pypa/pip/blob/main/src/pip/_internal/cli/cmdoptions.py#L294
I'll still take 30 seconds over 300 😄
There was a problem hiding this comment.
I'd really like to have some data on this what real world values are
4 participants
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.