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

Implement proxy_form_env() function #1998

Closed
Fuyukai opened this issue Jun 21, 2017 · 10 comments
Closed

Implement proxy_form_env() function #1998

Fuyukai opened this issue Jun 21, 2017 · 10 comments
Labels
Milestone

Comments

@Fuyukai
Copy link

Fuyukai commented Jun 21, 2017

Long story short

Custom keyword arguments are not provided to the request class constructor by the _request method on the client session. This prevents certain arguments from being provided (e.g. proxy_from_env) as there's no arguments on the request method for them.

Expected behaviour

They would be passed.

Actual behaviour

They aren't.

Steps to reproduce

async def something():
    async with aiohttp.ClientSession() as sess:
        async with sess.get("https://httpbin.org/ip", proxy_from_env=True) as r:
            return (await r.json())

TypeError: _request() got an unexpected keyword argument **'proxy_from_env'**

Your environment

aiohttp 2.2.0 on Python 3.5.3

@asvetlov
Copy link
Member

Please don't do it.
If you need to get proxy settings from environment variables -- use explicit function:

def proxy_from_env():
    proxy, proxy_auth = _parse(os.environ)
    return {'proxy': proxy, 'proxy_auth': proxy_auth}

async def something():
    async with aiohttp.ClientSession() as sess:
        async with sess.get("https://httpbin.org/ip", **proxy_from_env()) as r:
            return (await r.json())

BTW if you would to make a PR for adding proxy_from_env() function to aiohttp -- you are welcome.

@fafhrd91
Copy link
Member

@asvetlov
Copy link
Member

Ooops, I've missed this.
So what is the issue?

@Fuyukai
Copy link
Author

Fuyukai commented Jun 22, 2017

we load proxy info from env already.

Only if proxy_from_env=True is passed to ClientRequest - which it can't be, because there's no way to pass extra keyword arguments through _request.

@asvetlov
Copy link
Member

asvetlov commented Jun 22, 2017

I see. The feature is not completed.
@fafhrd91 maybe it's time to remove it and replace with proxy_from_env() function?
ClientRequest is in gray zone anyway, we shouldn't support backward compatibility for it.

@fafhrd91
Copy link
Member

+1

@asvetlov asvetlov changed the title Keyword arguments are not passed to _request_class in _request Implement proxy_form_env() function Jun 26, 2017
@asvetlov asvetlov added the good first issue Good for newcomers label Jun 30, 2017
@asvetlov asvetlov added this to the 2.3.0 milestone Jun 30, 2017
@GDICommander
Copy link

Since the feature is not complete, it would be useful for current users of aiohttp to remove the mention of proxy_from_env from the Proxy support section of this page: http://aiohttp.readthedocs.io/en/stable/client.html#proxy-support

@asvetlov
Copy link
Member

asvetlov commented Sep 8, 2017

Unfortunately my proposal doesn't work. There are two proxies: HTTP and HTTPS, proxy_from_env should either return a dict with both http/https keys or accept desired schema name.
First solution requires a new param to client api, second is very unconvenient.

Thus let's add bool proxy_from_env param to client.request() and others as mentioned in doc and intended by first incomplete implementation.

@asvetlov
Copy link
Member

asvetlov commented Sep 8, 2017

Actually it should be parameter for ClientSession() constructor, if user need pass different proxy settings for different request -- hi should do it explicitly.

asvetlov added a commit that referenced this issue Sep 17, 2017
@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants