Skip to content
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

Replace RoundTripper in favour of custom NewRequest method #477

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

TheoBrigitte
Copy link
Member

@TheoBrigitte TheoBrigitte commented Dec 20, 2024

Getting rid of the RoundTripper as it should not modify the request because its values might be read concurrently, see https://pkg.go.dev/net/http#RoundTripper

Proposing an alternative solution which uses NewRequest method to build the request and set the X-Scope-OrgID: tenantId header if needed.

RoundTripper should not modify the request as its values
might be read concurrently.
@TheoBrigitte TheoBrigitte requested a review from a team as a code owner December 20, 2024 12:05
@TheoBrigitte TheoBrigitte self-assigned this Dec 20, 2024
@QuentinBisson
Copy link
Contributor

I don't specifically think we need this though as this was documented more than 10 years ago and is not causing us any issues.

@TheoBrigitte
Copy link
Member Author

I don't specifically think we need this though as this was documented more than 10 years ago and is not causing us any issues.

The documentation is still valid and the change is fairly recent, this could cause problems.

@QuentinBisson
Copy link
Contributor

In the context of thousands of goroutines I would agree, but in an operator that reconciles things in order, i'm not sure but as you wish

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants