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

Mount API #977

Closed
tomchristie opened this issue May 21, 2020 · 2 comments · Fixed by #1362
Closed

Mount API #977

tomchristie opened this issue May 21, 2020 · 2 comments · Fixed by #1362
Labels
enhancement New feature or request requests-compat Issues related to Requests backwards compatibility
Milestone

Comments

@tomchristie
Copy link
Member

Somewhat related to #927, #954

We now have a really nicely defined Transport API. We also have the ability to mount proxies under particular schemes, or particular hosts.

Really we ought to be generalising that into a "mount a transport onto this scheme/domain/...".

Eg...

client.mount('http:', ...)  # Mount a particular transport instance to handle all "http" scheme requests.

There's an equivalent API in requests... https://requests.readthedocs.io/en/master/user/advanced/#transport-adapters

>>> s = requests.Session()
>>> s.mount('https://github.com/', MyAdapter())

Right now the closest we have is proxies={<mount-point>: <Proxy or Transport>}. However the set of mount points that are supported doesn't feel super well defined, and it nominally only supports proxies (even through they can actually be any transport instance).

Here's a bit of a proposal for how a more consistent, and generalised mount API might look...

  • Support a .transports = {'http': ..., 'https': ...} read-only property on the client.
  • The default transport sets 'http' and 'https'.
  • Transports can only be mounted while a client is closed. We'll track that status in .send/.close.
  • Closing a client closes all the transports.
  • We'd support .mount(prefix, transport).

Glossing over a bunch of stuff here since I just want to get the idea out there.

Ideally we'd also move away from having proxy transport instance be supported via proxies = ..., and instead only support the simple proxy config style for proxies=..., with any more specific transport mounting handled via the .mount interface so that there's only one way to do things.

@tomchristie tomchristie added this to the v1.0 milestone May 21, 2020
@tomchristie
Copy link
Member Author

Although this is new proposed API, I think it falls under the 1.0 cutoff, because it impacts stuff we want to resolve for 1.0.

@tomchristie tomchristie mentioned this issue Jul 16, 2020
@tomchristie tomchristie added the enhancement New feature or request label Jul 27, 2020
This was referenced Jul 27, 2020
@tomchristie
Copy link
Member Author

Since #1098, #1099, #1103, this has now become more obvious.

I don't think we should support a .mount() method - that's awkward because really you only want to mount or unmount transports on instantiating the client, so that you can ensure that the transport lifespans match the client lifespan.

Instead the API ought to look like this...

client = httpx.Client(mounts={"http://www.example.com": WSGITransport(...)})
client = httpx.Client(mounts={"http://": MyCustomTransport()})
client = httpx.Client(mounts={"http://": MyCustomTransport(), "https://": MyCustomTransport()})
client = httpx.Client(mounts={"all://": MyCustomTransport()})

Additionally we could extend our URL pattern matching to also support the following...

client = httpx.Client(mounts={"http://www.example.com/some/path/prefix": WSGITransport(...)})

At some point we might also want to think about exposing our pattern -> transport info from the client, since that'll help developers better understand what proxies and mounts have been introduced, and get a better internal model of how transport lookups operate.

For example, perhaps we'd end up with something like this at some point...

>>> os.environ["NO_PROXY"] = "example.com"
>>> os.environ["HTTP_PROXY"] = "localhost:1234"
>>> client = httpx.Client(mounts={"https://www.google.com/some/path": httpx.WSGITransport(...)})
>>> client.get_transports()
{
    <httpx.URLPattern("https://www.google.com/some/path")>: httpx.WSGITransport(...),
    <httpx.URLPattern("all://*example.com")>: httpx.SyncConnectionPool(...),
    <httpx.URLPattern("all://localhost:1234")>: httpx.SyncHTTPProxyl(...),
    <httpx.URLPattern("all://")>: httpx.SyncConnectionPool(...),
}

(Note that I've renamed URLMatcher in this example case, since I think httpx.URLPattern might be a better name)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request requests-compat Issues related to Requests backwards compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant