Skip to content

On proxies={"http": ...} vs. proxies={"http://": ...} #1105

Closed

Description

@lovelydinosaur

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.

Activity

  1. added this to the v0.14 milestone on Jul 31, 2020
  2. cdeler commented on Aug 4, 2020

    @cdeler
    Contributor

    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 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

  3. added 7 commits that reference this issue on Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    user-experienceEnsuring that users have a good experience using the library

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions