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

fix: don't use configured proxy for unix socket #7

Merged
merged 3 commits into from
Feb 19, 2024

Conversation

ae-govau
Copy link
Contributor

Without this, this module won't work when https_proxy / http_proxy environment variables are set, as these are picked up by the default Go transport clients, and it erroneously tries to the send the connecting through the proxy.

We fix by explictly niling the .Proxy setting after cloning the net.Transport.

@ae-govau
Copy link
Contributor Author

ae-govau commented Feb 6, 2024

@peterbourgon - would you kindly have a chance to consider accepting this change? It's a one-liner, and allows this library to function correctly when run in an environment with a corp proxy.

Copy link
Owner

@peterbourgon peterbourgon 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. Sorry for missing it originally. It took me a moment to understand what was going on here, I made a few suggestions informed by my journey to understanding. Accept those and I'm happy to merge.

ae-govau and others added 2 commits February 7, 2024 11:32
Co-authored-by: Peter Bourgon <[email protected]>
Co-authored-by: Peter Bourgon <[email protected]>
@ae-govau
Copy link
Contributor Author

ae-govau commented Feb 7, 2024

Thanks @peterbourgon - I've accepted the suggestions. We appreciate your library, thanks for maintaining it.

@ae-govau
Copy link
Contributor Author

Hi @peterbourgon - would you kindly be able to merge this PR and cut a new release? I accepted the changes which you suggested.

Thanks!

@peterbourgon
Copy link
Owner

peterbourgon commented Feb 19, 2024

Oops! Apologies, fell out of my queue.

https://github.com/peterbourgon/unixtransport/releases/tag/v0.0.4

@peterbourgon peterbourgon merged commit 10197ac into peterbourgon:main Feb 19, 2024
2 checks passed
@peterbourgon
Copy link
Owner

@ae-govau Please let me know if that doesn't work for you for any reason.

@ae-govau
Copy link
Contributor Author

Thanks @peterbourgon !

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