Configurable http client timeouts for alioss/gcs/s3 - #156
Conversation
|
Is the new http_client_timeout a global timeout used e.g. for the complete blob upload/download? For fog, the default excon timeouts were (see https://github.com/excon/excon/blob/master/README.md#timeouts-and-retries):
|
* allow configuration of http client timeouts * if nothing is configured, no http client timeout applies
d972924 to
ec839ff
Compare
|
To give some more context: These are the current Golang default settings: Go's http.DefaultClient / http.DefaultTransport has: This is sufficient to detect dead TCP connections and closely resembles the behavior of the old Fog/excon libraries. This PR allows the configuration of http client request timeouts to detect slow or unresponsive http servers. Note that large blobs are chunked in smaller fragments and the request timeout applies to one chunk. The sizes depend on the hyperscaler SDK used. |
| if requestTimeout <= 0 { | ||
| return 0, ErrNonPositiveHTTPRequestTimeout | ||
| } | ||
|
|
There was a problem hiding this comment.
No round up check in here as in alioss config
There was a problem hiding this comment.
GCS uses time.Duration directly, which supports sub-second precision.
| if httpRequestTimeout <= 0 { | ||
| return 0, errorNonPositiveHTTPRequestTimeout | ||
| } | ||
|
|
There was a problem hiding this comment.
No round up check in here as in alioss config
There was a problem hiding this comment.
S3 uses time.Duration directly, which supports sub-second precision.
|
|
||
| if _, err := strconv.ParseFloat(c.HTTPRequestTimeout, 64); err == nil { | ||
| return 0, fmt.Errorf("invalid http_request_timeout: missing duration unit") | ||
| } |
There was a problem hiding this comment.
I guess this check should be also in other clients config
There was a problem hiding this comment.
S3 and GCS use time.ParseDuration which catches wrong formats. Alioss is different and expects full seconds as int64.
No description provided.