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

Fixing missed arguments for pools when init transports #2997

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

NewUserHa
Copy link

Summary

class AsyncHTTPTransport(AsyncBaseTransport):
def __init__(
self,
verify: VerifyTypes = True,
cert: typing.Optional[CertTypes] = None,
http1: bool = True,
http2: bool = False,
limits: Limits = DEFAULT_LIMITS,
trust_env: bool = True,
proxy: typing.Optional[Proxy] = None,
uds: typing.Optional[str] = None,
local_address: typing.Optional[str] = None,
retries: int = 0,
socket_options: typing.Optional[typing.Iterable[SOCKET_OPTION]] = None,
) -> None:
ssl_context = create_ssl_context(verify=verify, cert=cert, trust_env=trust_env)
if proxy is None:
self._pool = httpcore.AsyncConnectionPool(
ssl_context=ssl_context,
max_connections=limits.max_connections,
max_keepalive_connections=limits.max_keepalive_connections,
keepalive_expiry=limits.keepalive_expiry,
http1=http1,
http2=http2,
uds=uds,
local_address=local_address,
retries=retries,
socket_options=socket_options,
)
elif proxy.url.scheme in ("http", "https"):
self._pool = httpcore.AsyncHTTPProxy(
proxy_url=httpcore.URL(
scheme=proxy.url.raw_scheme,
host=proxy.url.raw_host,
port=proxy.url.port,
target=proxy.url.raw_path,
),
proxy_auth=proxy.raw_auth,
proxy_headers=proxy.headers.raw,
ssl_context=ssl_context,
max_connections=limits.max_connections,
max_keepalive_connections=limits.max_keepalive_connections,
keepalive_expiry=limits.keepalive_expiry,
http1=http1,
http2=http2,
socket_options=socket_options,
)
elif proxy.url.scheme == "socks5":
try:
import socksio # noqa
except ImportError: # pragma: no cover
raise ImportError(
"Using SOCKS proxy, but the 'socksio' package is not installed. "
"Make sure to install httpx using `pip install httpx[socks]`."
) from None
self._pool = httpcore.AsyncSOCKSProxy(
proxy_url=httpcore.URL(
scheme=proxy.url.raw_scheme,
host=proxy.url.raw_host,
port=proxy.url.port,
target=proxy.url.raw_path,
),
proxy_auth=proxy.raw_auth,
ssl_context=ssl_context,
max_connections=limits.max_connections,
max_keepalive_connections=limits.max_keepalive_connections,
keepalive_expiry=limits.keepalive_expiry,
http1=http1,
http2=http2,
)
else: # pragma: no cover

eg.

  • in AsyncHTTPTransport, AsyncConnectionPool has the argument of retries passed in, but AsyncHTTPProxy and AsyncSOCKSProxy have not;
  • in HTTPTransport, it has the same issue.

discussion: #2988

This also requires a fix in https://github.com/encode/httpcore

Checklist

  • 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.

@NewUserHa
Copy link
Author

The PR requires encode/httpcore#857 PR to pass checks

@tomchristie
Copy link
Member

The PR requires encode/httpcore#857 PR to pass checks

Can we just start by considering retries here? That's already supported by httpcore>1.0, right?

@NewUserHa
Copy link
Author

Ok, done.

Should open a new PR for the rest missed arguments?

@tomchristie
Copy link
Member

Okay, first up let's just have a sanity check... do connection retries actually get applied when connecting to proxies?

@NewUserHa
Copy link
Author

The SocksProxy?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants