Fixing missed arguments for pools when init transports

[NewUserHa]

Summary

related to PR: https://github.com/encode/httpx/pull/2997

Checklist

• [X] I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
• [ ] I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
• [ ] I've updated the documentation accordingly.

[karpetrosyan]

Let's start with an understanding of what a proper signature for http proxy connections is.

Here is the signature of AsyncForwardHTTPConnection class.

class AsyncForwardHTTPConnection(AsyncConnectionInterface):
    def __init__(
        self,
        proxy_origin: Origin,
        remote_origin: Origin,
        proxy_headers: Union[HeadersAsMapping, HeadersAsSequence, None] = None,
        keepalive_expiry: Optional[float] = None,
        network_backend: Optional[AsyncNetworkBackend] = None,
        socket_options: Optional[Iterable[SOCKET_OPTION]] = None,
        proxy_ssl_context: Optional[ssl.SSLContext] = None,
    ) -> None:

I believe we are missing a few parameters here, as it is simply a wrapper for the AsyncHTTPConnection, which has the following signature:

class AsyncHTTPConnection(AsyncConnectionInterface):
    def __init__(
        self,
        origin: Origin,
        ssl_context: Optional[ssl.SSLContext] = None,
        keepalive_expiry: Optional[float] = None,
        http1: bool = True,
        http2: bool = False,
        retries: int = 0,
        local_address: Optional[str] = None,
        uds: Optional[str] = None,
        network_backend: Optional[AsyncNetworkBackend] = None,
        socket_options: Optional[Iterable[SOCKET_OPTION]] = None,
    ) -> None:

The constructor of AsyncForwardHTTPConnection contains the following code:

        self._connection = AsyncHTTPConnection(
            origin=proxy_origin,
            keepalive_expiry=keepalive_expiry,
            network_backend=network_backend,
            socket_options=socket_options,
            ssl_context=proxy_ssl_context,
        )
        self._proxy_origin = proxy_origin
        self._proxy_headers = enforce_headers(proxy_headers, name="proxy_headers")
        self._remote_origin = remote_origin

So we need all of the arguments from AsyncHTTPConnection PLUS proxy_origin, remote_origin, proxy_ssl_context, and proxy_headers for the proxy logic in AsyncForwardHTTPConnection.

Here are the arguments that the proxy class lacks:

• http1
• http2
• retries
• local_address
• uds

Also, in the ForwardHTTPConnection, we could change the argument proxy_ssl_context to ssl_context because, unlike the TunnelHTTPConnection, we only have one ssl layer, so proxy_ssl_context is, in my opinion, a little bit ambigious here.

[lovelydinosaur]

Understood, thanks for this @NewUserHa. Looks like there's some unintended reorderings here. If those can be removed we can get a lower footprint to this PR, which will make it more neatly reviewable.

[NewUserHa]

The HttpProxy and Socks5Proxy only need either proxy_url or uds, should default proxy_url to ""?

[karpetrosyan]

In this PR, we can simply enforce the ordering of retries, local_address, and uds arguments, and open another for adding retries to Proxy.

For uds support in proxy classes, let's just open a discussion first.

[Panda1283]

The HttpProxy and Socks5Proxy only need either proxy_url or uds, should default proxy_url to ""?

[NewUserHa]

discussion for uds support in proxy classes: https://github.com/encode/httpcore/discussions/858

[NewUserHa]

done. now this PR only:

• added lacked retries, local_address, uds, socket_options parameters and corresponding doc strings;
• and reordered these parameters for consistency.

[NewUserHa]

the SocksProxy still lacks local_address, uds; And retries is in the class only but Socks5Connection does not use it, can open another PR for these.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.