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

Update sessions.py #5982

Closed
wants to merge 1 commit into from
Closed

Update sessions.py #5982

wants to merge 1 commit into from

Conversation

junqfisica
Copy link

This change will make the hierarchy request **kwargs -> Session args -> environment to be respected. This also solves problems with pip as discussed here and #5735

This change will make the hierarchy request **kwargs -> Session args -> environment to be respected. This also solves problems with pip as discussed [here](pypa/pip#9691 (comment))  and  psf#5735
@sigmavirus24
Copy link
Contributor

Read #5735 more closely please. There are certainly people who are unknowingly relying on this behaviour in 2.x and we cannot break them suddenly. It's worse because even if we document this they will still be surprised as they likely have no understanding that they're affected

@junqfisica
Copy link
Author

@sigmavirus24 Thank you for the comment but I disagree this is a wanted behavior. The current status of Session breaks the usage of pip when behind a proxy and the change I proposed is not the same as in #5735. My change keeps the hierarchy the way it should be as already mentioned in the documentation.

The biggest problem right now is that when you use

pip install package --proxy=http\\:proxy_url

The way they inject proxy to request is by creating a session object and setting the proprety proxies to it:

from requests import Request, Session

proxies = { "http"  : http_proxy, "https" : https_proxy}
session = Session()
session.proxies = proxies
session.request(url)

The piece of code above doesn't work as expected in many proxy environments, because Request is actually overriding the given proxies by the system proxies and due to changes in urlib3 it doesn't work if you are doing an HTTPS request using an HTTP proxy server. Many developers can't directly change environment proxy. Thus, due to this problem newer versions of pip (> 20.2.x) can't install packages using the proxy.

I agree that pip could solve that by giving the proxy directly to request like:

from requests import Request, Session

proxies = { "http"  : http_proxy, "https" : https_proxy}
session = Session()
session.proxies = proxies
session.request(url, proxies)

Although, since you allow to set proxies like: session.proxies = proxies that SHOULD be respected.
Saying that I don't see how my proposed change would break anything, since I pinpoint the problem of overriding the proxy. I would please ask for careful consideration on this change.

@sigmavirus24
Copy link
Contributor

Thank you for the comment but I disagree this is a wanted behavior.

Never said wanted. I said unknowingly relied upon. An application is working today that very well may break in confusing and unexpected ways after releasing this. The behaviour is wrong, we intend to fix it in a deliberately breaking release when we can dedicate time towards such a development effort. It's not something we can fix in the 2.x release series.

This change has been carefully considered for as long as the original PR has been open.

@psf psf locked as resolved and limited conversation to collaborators Nov 23, 2021
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.

2 participants