-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
Remove usage of http.DefaultClient #3532
Conversation
Thanks for this... Though I am not sure I understand the motivation. Was this discussed somewhere else? |
@apparentlymart Yes, it spun off of an internal discussion. I'm on a crusade to remove usage of http.DefaultClient as it very easily causes strange and hard-to-diagnose problems like hashicorp/vault#700. As http.DefaultClient is literally just a The next step in Terraform will be going through dependencies and ensuring that, where possible, code paths in dependencies that use DefaultClient are instead given their own clients. I'll submit a PR for that later today. Tests should certainly be run to ensure that nothing was relying on this behavior, but in that case the right fix isn't to use DefaultClient, it's to pass in the properly-configured client to the library for usage. |
…eam merging to completely solve this.
c2fd0c2
to
b0ceffc
Compare
Hold off on merging this for the moment; there is an upstream fix going into packngo, and once it does I'll update this. |
👍 This worked well as a fix for hashicorp/vault#700 |
Done -- ready to merge! |
Remove usage of http.DefaultClient
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
No description provided.