Internal transport discussion

[theacodes]

This library needs to satisfy two orthogonal goals related to HTTP:

1. It needs to be able to make HTTP requests internally (e.g., in refresh())
2. It needs to allow users to attach credentials to an HTTP object of their choosing.

If there were only one Python HTTP library this would all be trivial. If the library was called unicorn, we could just use unicorn.request directly everywhere internally for (1). For (2), we would just write a single function to attach the credentials to unicorn.

This discussion is exclusively about (1), but care should be made not to limit our options for (2).

Use Case (1)

Use case (1) boils down to the need for a consistent way to make an http request and get response information. This use-case is only for library internals.

What's done today

transport.request takes any http object and then uses isinstance to figure out which transport should be used to call. Basically:

# Calling code
credentials.refresh(http, ...)

# credentials.py
def refresh(http):
    ...
    r = transport.request(http, ...)
    ...

# transport.py
def request(http, method, uri, ...):
    transport_module = get_transport_for_http(http)
    return transport_module.request(http, method, uri, ...)

# urllib3_transport.py
def request(http, method, uri, ...):
    return http.request(method, uri)

The indirection has the benefit that we can accept any supported http object at the call sites of methods like refresh(). The drawback is the indirection and the cost of the lookup for every request.

What we could do

Use http.client

We could easily use the built-in http.client - but urlilb3 and requests have significant benefits, and properly doing ssl with http.client can be error prone. It also leads us to situation where the http client the user is using and the http client we're using can be different.

Set credentials.http

Presently credentials are completely independent of http. Any methods that need to make requests must accept an http parameter. This proposes adding credentials.http that is set to a new private interface transport._HTTP. None of the request-making methods need http as an argument anymore.

This has the distinct drawback that credentials can now only be associated with a single http object. It also introduces a new interface (and subclasses for every transport).

Just pass in transport_impl.request

Instead of passing in an http object and letting the lookup happen, make the argument to refresh() et al be an interface that matches transport.request like this:

def request(method, uri, ...)

Since the caller of refresh knows the transport implementation that's currently being used it can just pass in, for example, functools.partial(urlilb3_transport.request, bare_http). The call stack would then look like this:

# Calling code
bound_request = functools.partial(urllib3_transport.request, self)
credentials.refresh(bound_request)

# credentials.py
def refresh(request):
    ...
    r = request(...)
    ...

# urllib3_transport.py
# http provided by functools.partial above.
def request(http, method, uri, ...):
    return http.request(method, uri)

This has a lot of benefits:

1. The interface for request can be made public without making any other part of our transport implementation public.
2. No lookup penalty.
3. refresh() et al will have well defined argument types.
4. Gives implementors of use case (2) the option of passing in something else for request. :)

[theacodes]

@dhermes

[dhermes]

@jonparrott the "Bind credentials to http" describes binding an http to credentials

[theacodes]

@dhermes updated.

[theacodes]

Resolved, at least for use case (1).