-
Notifications
You must be signed in to change notification settings - Fork 386
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
feat: add support for basic auth, insecure tls and headers #5061
feat: add support for basic auth, insecure tls and headers #5061
Conversation
2a0f2ef
to
effb5fb
Compare
internal/http/download.go
Outdated
// This is a one-shot HTTP client which should release resources immediately. | ||
DisableKeepAlives: true, | ||
}, | ||
transport := http.DefaultTransport.(*http.Transport).Clone() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a deliberate decision not to use the default transport here, as it defines a bunch of timeouts that make configuration difficult. I think for this simple use case the staleness timeout is all that's needed.
The only thing that's probably good to set here is ProxyFromEnvironment
. I'd even say setting this here should actually be backported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the proxy issue I have opened #5064, once that is merged I can rebase this one and get rid of the Clone() call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that #5064 is merged I have rebased this to avoid cloning the default transport. Let me know what you think.
a5b654a
to
e4abd28
Compare
This pull request has merge conflicts that need to be resolved. |
add suport for basic auth, insecure tls and headers to the download package. Signed-off-by: Ricardo Maraschini <[email protected]>
e4abd28
to
036aad8
Compare
Description
add suport for basic auth, insecure tls and headers to the download package. this PR is part of an adr implementation that i plan to introduce gradually.
Fixes # (issue)
Type of change
How Has This Been Tested?
Checklist: