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

Provide defaults for connection pool #1173

Closed
bwelling opened this issue Aug 11, 2020 · 2 comments
Closed

Provide defaults for connection pool #1173

bwelling opened this issue Aug 11, 2020 · 2 comments
Labels
discussion user-experience Ensuring that users have a good experience using the library

Comments

@bwelling
Copy link

https://github.com/encode/httpx/blob/master/docs/advanced.md#custom-transports provides an example for configuring an httpx client with a custom connection pool, which is needed to specify a local address. This example is overly complicated and fragile, though, as it requires far too much knowledge about the underlying httpcore interface.

Specifically, if a user wants standard client behavior except for one small thing, that user is required to set a bunch of unrelated client parameters which have no visible defaults, as those defaults would typically be set by the httpx library code. The values of some of these parameters will likely change over time, as will the set of parameters passed by the standard httpx code. So, anything using the custom transport paradigm would either diverge, or the user would need to periodically poll httpx to update to the current set.

In a comment on encode/httpcore#100, I suggested that providing wrappers in httpx around httpcore.SyncConnectionPool and httpcore.AsyncConnectionPool that merge a set of user-provided configuration options with the defaults, and @tomchristie responded with We could certainly think about switching our policy on httpcore defaults, yup..

It would certainly be a lot easier (as well as safer, and more future-proof) if code could do something like:

transport = httpcore.SyncConnectionPool(local_address="0.0.0.0")
client = httpx.Client(transport=transport)

if they wanted everything to operate as an httpx client normally would, except with a custom local address (that may not be the right interface, but the point is that it doesn't require specifying all of the other things that should be unchanged).

@florimondmanca
Copy link
Member

florimondmanca commented Aug 12, 2020

Note that for the problem you've described ("as an HTTPX user, give me a standard transport except with a fews knobs tweaked"), an alternative solution that would keep the responsibility of setting defaults in the realm of HTTPX and higher-level clients would be something like this:

import httpx

transport = httpx.init_transport(local_address="0.0.0.0")  # Not sure about naming or namespacing here.
client = httpx.Client(transport=transport)

There is currently a private method on httpx.Client that basically does this, except it hasn't been updated for allowing local_address yet:

httpx/httpx/_client.py

Lines 541 to 566 in a4463d0

def _init_transport(
self,
verify: VerifyTypes = True,
cert: CertTypes = None,
http2: bool = False,
limits: Limits = DEFAULT_LIMITS,
transport: httpcore.SyncHTTPTransport = None,
app: typing.Callable = None,
trust_env: bool = True,
) -> httpcore.SyncHTTPTransport:
if transport is not None:
return transport
if app is not None:
return WSGITransport(app=app)
ssl_context = create_ssl_context(verify=verify, cert=cert, trust_env=trust_env)
return httpcore.SyncConnectionPool(
ssl_context=ssl_context,
max_connections=limits.max_connections,
max_keepalive_connections=limits.max_keepalive_connections,
keepalive_expiry=KEEPALIVE_EXPIRY,
http2=http2,
)

Note how it's basically a static method. We've kept in the Client namespace mostly for implementations reasons, but it doesn't have to be that way if there's a legit user need.

I know @tomchristie has been thinking about how HTTPX could expose users more to the concept of transports. So perhaps exposing something equivalent to Client._init_transport() as public API could fit in that area.

Edit: I think we can move this discussion to the HTTPX issue tracker.

@florimondmanca florimondmanca transferred this issue from encode/httpcore Aug 12, 2020
@florimondmanca florimondmanca added discussion user-experience Ensuring that users have a good experience using the library labels Aug 12, 2020
@tomchristie
Copy link
Member

Resolved now that httpx.HTTPTransport() exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion user-experience Ensuring that users have a good experience using the library
Projects
None yet
Development

No branches or pull requests

3 participants