- Notifications
You must be signed in to change notification settings - Fork1.1k
fix: remove initial global HTTP client usage#20128
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This commit is an effort to further isolate tests by removingshared used of the global default HTTP client.
Make easy, low-risk removals of default HTTP client usage.Some application code calls CloseIdleConnections on the default HTTPclient which can cause other code under test using the default HTTPclient to be impacted. This should reduce test flakiness.
github-actionsbot commentedOct 1, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
All contributors have signed the CLA ✍️ ✅ |
zedkipp commentedOct 1, 2025
I have read the CLA Document and I hereby sign the CLA |
buenos-nachos left a comment
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.
Everything looks good to me, but I'd rather this get a second opinion from someone with more backend context
(Also, for future reference, Kayla and I got tagged as code owners because the PR changed a file in/site, but the main intent for that check is to get more eyes on the TypeScript parts of the codebase. Kayla can absolutely hold her own when it comes to backend stuff – I'm definitely still getting my feet wet, though, so I'm going to be less useful. I'd say that if you're not updating any TypeScript code, you can go ahead and tag any backend engineer for changes like these.)
bcpeinhardt commentedOct 2, 2025
Nice 😎 It looks good to me. I went looking for other types of invocations that might use the default client (like Might be a good opportunity to start documenting some testing practices in the CONTRIBUTING guide (using |
4d1003e intomainUh oh!
There was an error while loading.Please reload this page.
This PR makes the initial steps at removing usage of the global Go HTTP client, which was seen to have impacts on test flakiness incoder/internal#1020. The first commit removes uses from tests, with the exception ofone test that is tightly coupled to the default client. The second commit makes easy/low-risk removals from application code (I'm new to the project). This should have some impact to reduce flakiness.