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

Check NO_PROXY env to disable proxy settings #11497

Closed
wants to merge 1 commit into from
Closed

Check NO_PROXY env to disable proxy settings #11497

wants to merge 1 commit into from

Conversation

q0w
Copy link
Contributor

@q0w q0w commented Oct 7, 2022

Fix #5378

@SpecLad
Copy link
Contributor

SpecLad commented Oct 11, 2022

This isn't how no_proxy normally works. It's supposed to contain a list of domains for which a proxy is not to be used.

@pfmoore
Copy link
Member

pfmoore commented Oct 11, 2022

Also, doesn't requests pick up the NO_PROXY environment variable automatically? We shouldn't need any explicit code in pip for this. The discussion in #5378 seems pretty confused - I can't see anyone who tried the NO_PROXY environment variable1 and confirmed it wasn't working.

As far as I can see, the expectation from the issue was for a --no-proxy option to pip that would disable any proxy settings. I'm not entirely sure how it should work, though. Should it just ignore any pip-specific proxy settings like --proxy whether specified via an option, a PIP_PROXY environment variable, or a config file? Should it explicitly remove the HTTP_PROXY, HTTPS_PROXY and NO_PROXY environment variables from the environment? As well, or instead?

Footnotes

  1. At least, not correctly. Someone tried setting it to "1", but according to the documentation I can find via Google, it's supposed to be a list of servers which should be accessed without a proxy.

Copy link
Member

@pfmoore pfmoore left a comment

Choose a reason for hiding this comment

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

We should not use the NO_PROXY environment variable in a non-standard way like this.

@q0w

This comment was marked as resolved.

@q0w
Copy link
Contributor Author

q0w commented Oct 11, 2022

To not add a new optional --no-proxy argument, you can use NO_PROXY='*' to disable proxy for all domains, why not?
https://github.com/encode/httpx/blob/36f16234bc47006c7efd1b8d4b363000ab8b2b3d/tests/client/test_proxies.py#L248

@q0w

This comment was marked as outdated.

@q0w
Copy link
Contributor Author

q0w commented Oct 11, 2022

I've tested manually NO_PROXY='*' and it works out of box with pip for disabling proxy without this patch.

@pfmoore
Copy link
Member

pfmoore commented Oct 11, 2022

I've tested manually NO_PROXY='*' and it works out of box with pip for disabling proxy without this patch.

So I guess that means there's a solution for #5378 without needing any change to pip?

@q0w
Copy link
Contributor Author

q0w commented Oct 11, 2022

Yes, it just needs to be documented
but it would not take precedence over --proxy arg

@pfmoore
Copy link
Member

pfmoore commented Oct 11, 2022

https://pip.pypa.io/en/stable/user_guide/#using-a-proxy-server states

by setting the standard environment-variables http_proxy, https_proxy and no_proxy.

I don't think it's up to us to document how those variables work, as they are intended to work the same as in every other application. But I wouldn't object to a brief clarifying statement, nor would I object to a note about how they relate to --proxy. Are you interested in re-purposing this PR for such a documentation change?

@q0w
Copy link
Contributor Author

q0w commented Oct 11, 2022

Sorry, i am not good at writing docs.

@uranusjr
Copy link
Member

Either way I think this PR can be closed?

@q0w q0w closed this Oct 12, 2022
@q0w
Copy link
Contributor Author

q0w commented Oct 13, 2022

Or I can reimplement this by adding --noproxy arg for pip. @pfmoore what do you think?

@pfmoore
Copy link
Member

pfmoore commented Oct 13, 2022

Or I can reimplement this by adding --noproxy arg for pip. @pfmoore what do you think?

Given that the NO_PROXY environment variable handles this, I don't see the benefit.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to bypass http proxy
4 participants