feat: introduce flag to set the http response timeout value #185
Conversation
- It is easier to see what has been 'changed' from the DefaultTransport. Authored-by: Dennis Leon <leonde@vmware.com>
| InsecureSkipVerify: (opts.VerifyCerts == false), | ||
| }, | ||
| }, nil | ||
| clonedDefaultTransport := http.DefaultTransport.(*http.Transport).Clone() |
There was a problem hiding this comment.
the only downside i see here is that if something changes default transport this code will be affected. do we know if anything changing default transport anywhere?
There was a problem hiding this comment.
the upside is that as we upgrade the http package, we get any changes / updates made to the DefaultTransport. (we avoid drifting from DefaultTransport)
i.e. since inlining the DefaultTransport, we drifted in 2 fields:
ForceAttemptHTTP2setting to trueDialContext.DualStackhas been deprecated
sounds like a trade-off to me. While the risk you outline is true, currently nothing is mutating the DefaultTransport in our codebase or any of our libraries.
My assumption is that the chance of drifting from stdlib is higher than the chance something will mutate DefaultTransport, and that's why I think this tradeoff is 'worth it'.
Another assumption is that we don't want to drift from stdlib because the go folk are making reasonable sane changes to the DefaultTransport that we want in imgpkg.
DennisDenuto
changed the title
- used to specify how long requests to the registry should wait for a http response (headers) from the registry. - increased the default timeout from 10s -> 30s Authored-by: Dennis Leon <leonde@vmware.com>
DennisDenuto
force-pushed
the
refactor-default-http-transport
branch
from
c264078
to
ac194f4
Compare
Authored-by: Dennis Leon leonde@vmware.com