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

Proxy related doc updates #5670

Merged
merged 4 commits into from Dec 4, 2020

Conversation

smarie
Copy link

@smarie smarie commented Nov 27, 2020

Hi, I have spent the past 15 years working in professional environments where the http/https proxy was the most annoying issue to myself and all my developer colleagues (in java, matlab, python...). A few years ago I even ended up writing a internal tutorial for our corporate proxy, a public one (https://smarie.github.io/develop-behind-proxy/), and a environment switching tool (https://smarie.github.io/env-switcher-gui/) that is used by many of us especially the ones switching from home office to work place regularly.

This introduction just to say that even if this is issue is now fairly trivial to me, I have been helping hundreds of colleagues over the year (and am still from time to time) so I believe that it is not yet easy to understand for most people.

requests is working perfectly fine with proxies. However the way it finds the proxy-related information is not very well documented, and the way one can override this information on a Session is quite un-pythonic (a plain dict, therefore possibly error-prone). Some work I did a few months ago using requests-oauthlib made me once again think that it would be extremely helpful to have a good helper function on the Session object.

This PR contains two things:

  • a Session.set_http_proxy method with associated unit tests
  • an update for the Proxies documentation section

I had one afterthought after completing this, maybe you'll find this relevant too: instead of providing just one set_http_proxy method, maybe it would be better to split it into two : set_http_proxy and set_https_proxy. Far more simple and elegant in my opinion, but I'll wait for your first feedback before modding anything here.

Thanks for your time and for maintaining this great lib!

@sigmavirus24
Copy link
Contributor

Hi @smarie ,

Thanks for sending this along. Unfortunately, Requests is under a feature freeze so we're not accepting any changes that introduce new API surface area. Documentation updates to make things clearer/more explicit are always welcome though.

@smarie
Copy link
Author

smarie commented Nov 27, 2020

Thanks @sigmavirus24 ! I had hoped that such helper would not be considered into this API freeze state :(

Ok then, I'll revert the file changes so as to only leave the documentation mod.

@smarie
Copy link
Author

smarie commented Nov 27, 2020

@sigmavirus24 : doc updated. Let me know what you think.

@smarie
Copy link
Author

smarie commented Nov 27, 2020

by the way if this is just doc I should probably remove my name from Authors.rst no?

@sigmavirus24 sigmavirus24 changed the title Proxy related doc updates and Session helper Proxy related doc updates Dec 4, 2020
@sigmavirus24 sigmavirus24 merged commit 5035827 into psf:master Dec 4, 2020
@smarie
Copy link
Author

smarie commented Dec 4, 2020

thanks @sigmavirus24

@richard-moss
Copy link

Hi all,

just found this issue, after struggling to find an answer to the (seemingly simple - before started digging) question of whether requests or urllib3 will use the OS level proxy configuration by default, if no specific proxy is specified. (my specific use case is an PY 3.8 app running on windows 10).

@smarie I read your (great btw!) docs you did up, and the updated requests doc, but they seem to suggest requests will only default to using the system env variables.

...but then I discovered a stackoverflow comment saying requests -would- use the OS proxy configuration.

I just posted a SO question related to this: https://stackoverflow.com/questions/65931275/python-requests-module-does-it-use-system-level-on-windows-proxy-settings

@smarie
Copy link
Author

smarie commented Jan 28, 2021

Thanks @richard-moss for your comment ! let's discuss on stackoverflow and if this comes to a definitive answer that can improve the doc in requests we can propose a change

@smarie
Copy link
Author

smarie commented Jan 28, 2021

I made a detailed answer in stackoverflow. If based on this answer you feel that the documentation of requests should be improved, please feel free to propose a change !

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.

None yet

3 participants