You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
On proxies={"http": ...} vs. proxies={"http://": ...} #1105
Currently we're in line with requests in supporting "all", "http", and "https" as valid proxy keys.
Internally we handle those as URL patterns that we can match against, and eg. "http" is actually mapped to "http://".
I'm wondering if we might have better internal consistency by...
Raising a warning on base usage of "all", "http", "https", later escalating this to an error.
Instead requiring an explicit "all://", "http://", "https://", all of which properly parse directly as URLs.
My feeling is that doing so would create a nicer consistency, and also has a subtle indication to developers about what is actually going on here.
For example...
proxies={"http://": httpx.Proxies(...)} neatly implies that proxies={"http://www.example.com": httpx.Proxies(...)} might also be a valid thing to do, which indeed it is.
Allowing proxies={"http": httpx.Proxies(...)} makes it less obvious that the key is a general purpose URL pattern.
Additionally this change means that other usages will end up with more expected behaviors, eg. proxies={"www.example.com": ...} will do what you'd expect, rather than mapping to proxies={"www.example.com://": ...} which is what we currently do. - Not true, that'd parse as scheme='', host='', path='www.example.com'. Instead we'd probably want to be strict and ensure that only absolute URLs are allowed as pattern keys. (While still including a graceful deprecation for plain "http", "https", "all")
This becomes doubly relevant once we introduce a mount API, which will use the same URL pattern style for it's API, as current proxies=... argument.
Are you talking about the changes in httpx._client.BaseClient._get_proxy_map (here: link)
Also, what should we do with get_environment_proxies? (link) This function requires envs like HTTPS_PROXY etc.
UPD: @tomchristie I've created a PR just to show my idea. If I'm wrong, feel free to discard it
Activity
cdeler commentedon Aug 4, 2020
Hello @tomchristie ,
Are you talking about the changes in
httpx._client.BaseClient._get_proxy_map(here: link)Also, what should we do with
get_environment_proxies? (link) This function requires envs likeHTTPS_PROXYetc.UPD: @tomchristie I've created a PR just to show my idea. If I'm wrong, feel free to discard it
encode#1105 raise warning if proxy scheme is http, https, all instead…
encode#1105 raise warning if proxy scheme is http, https, all instead…
encode#1105 raise warning if proxy scheme is http, https, all instead…
encode#1105 fixed mypy remarks
encode#1105 added unit tests for deprecation warning, raised when we …
encode#1105 added deprecation warning, raised when we try to use prox…
encode#1105 added deprecation warning, raised when we try to use prox…
Raise warning if proxy key is eg. "all" instead of "all://". (#1127)