Configurable http client timeout for Azure Blob Storage - #162
Configurable http client timeout for Azure Blob Storage#162jochenehret wants to merge 2 commits into
Conversation
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
c27e2cb to
64ea3e3
Compare
stephanme
left a comment
stephanme
left a comment
There was a problem hiding this comment.
Claude review mentioned that RequestTimeout is problematic for long running up/downloads and recommends to use ResponseHeaderTimeout instead so that connection/network problems are detected fast but long running data flow is not interrupted.
I remember that we discussed this and therefore decided to allow setting RequestTimeout but use an "infinite" default.
Maybe we should add ResponseHeaderTimeout with a 60s default. This would come closer to the fog/excon client behavior.
| } | ||
| return &blockblob.ClientOptions{ | ||
| ClientOptions: azcore.ClientOptions{ | ||
| Transport: &http.Client{Timeout: dsc.httpRequestTimeout}, |
There was a problem hiding this comment.
There is no transport set in the &http.Client{} which means that http.Client uses http.DefaultTransport instead of the default transport of the azure SDK.
http.DefaultTransport uses RenegotiateNever and has no HTTP/2 health-check pings. azcore's default transport uses RenegotiateFreelyAsClient, TLS 1.2 minimum, and ReadIdleTimeout: 10s for HTTP/2 pings. In environments with corporate proxies or Azure sovereign-cloud endpoints that request TLS renegotiation, all connections silently fail. The fix is to build a transport derived from azcore's defaults (via azidentity.DefaultTransportFromEnvironment or azcorehttp.NewDefaultTransport), set only the Timeout, and keep the RenegotiateFreelyAsClient config intact.
| } | ||
| } | ||
|
|
||
| func (dsc DefaultStorageClient) containerClientOptions() *azContainer.ClientOptions { |
There was a problem hiding this comment.
All 3 xxxClientOptions() methods have the same body. Could probably be simplified.
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
2 participants
No description provided.