Value of http2 not being passed on to create_ssl_context

[mborsetti]

I am not super-familiar with the innards of this repo, but in debugging something else I tangentially noticed from the log this entry, showing http2=False even though is generated when instantiating an object withhttpx.Client(http2=**True**):

load_ssl_context` verify=True cert=None trust_env=True http2=False

I have isolated this behavior to the two lines below, each calling create_ssl_context but without passing http2 as an argument, so http2 is defaulted to False.

httpx/httpx/_transports/default.py

Line 138 in f3eb3c9

ssl_context = create_ssl_context(verify=verify, cert=cert, trust_env=trust_env)

httpx/httpx/_transports/default.py

Line 279 in f3eb3c9

ssl_context = create_ssl_context(verify=verify, cert=cert, trust_env=trust_env)

I presume that create_ssl_context would not have been written with ahttp2 parameter unless it was useful, yet the http2 argument is never used in the code as these are the only two lines of code who call it.

Are those two lines correct or should http2 be passed onwards?

[lovelydinosaur]

Well spotted, you've found an unused appendix there... here's the deal...

There's two places where we need to know if HTTP/2 is enabled or not...

1. The SSL context needs to have the correct ALPN context.. ssl_context.set_alpn_protocols(["http/1.1", "h2"])
2. The connection pool needs to "know" if HTTP/2 is enabled or not upfront, so that we can properly determine if multiple outgoing requests to the same host should default to waiting for a single connection to open (HTTP/2, multiple streams) or default to opening multiple connection (HTTP/1.1).

This is actually a bit awkward... ideally we'd just set that in one place (the SSL context) and then have the connection pool determine the correct behavior based on that. However, there's no API for reading the ALPN context.

Currently we're mutating the SSL context within the connection pool to ensure that it has the correct ALPN context.

We can either...

• Remove the http2 config from the create_ssl_context, since it's currently unused there.
• Ensure that the SSL contexts that we create do have an interface for determining the ALPN context, and use that from the connection pool.

Ideally there's also some more comprehensive documentation for us to include here. I'd suggest that we look at resolving this once #3022 has landed.

[karpetrosyan]

I have already removed it in the #3022.
Is there something else we want to do here?

[lovelydinosaur]

No I think we're good there, just needs merging.