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

honor HTTPS_PROXY environment variable #1065

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

finncolman
Copy link
Contributor

This resolves issue 1064

@cla-assistant
Copy link

cla-assistant bot commented Sep 25, 2023

CLA assistant check
All committers have signed the CLA.

@cla-assistant
Copy link

cla-assistant bot commented Sep 25, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Finn Colman seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@finncolman
Copy link
Contributor Author

finncolman commented Sep 25, 2023

See: ProxyFromEnvironment

The DefaultTransport looks like this:

var DefaultTransport RoundTripper = &Transport{
	Proxy: ProxyFromEnvironment,

This is what is missing from this custom transport

Also, this fork has been tested in my system so I know it fixes the proxy issue.

@milindl milindl requested a review from rayokota October 3, 2023 09:22
@milindl
Copy link
Contributor

milindl commented Oct 3, 2023

Hi @rayokota , asking for your review here, as you would know this much better than I.

Maybe the proxy can be specified via the configuration (as a function?), rather than only being set to being picked from the environment.

Copy link
Member

@rayokota rayokota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @finncolman , LGTM

@rayokota rayokota merged commit a1daa4f into confluentinc:master Oct 19, 2023
1 check passed
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.

None yet

3 participants