Repository navigation
Advanced connection options. #88
Description
Prompted by a comment from @hynek
We ought to add more __init__ controls to AsyncConnectionPool(...) and SyncConnectionPool(...) for advanced connection options, including...
- UDS support. Add support for unix domain sockets httpx#511, Drop UDS support httpx#726
- System IP to use Allow the specifying of which system IP address to use for client connections. httpx#755
- Socket family and related options.
It'd be useful to do a comprehensive review of...
- What connection options are offered by asyncio?
- What connection options are offered by stdlib's standard sync networking API?
- What connection options are offered by trio?
Compare against controls available in urllib3, aiohttp.
Activity
hynek commentedon May 13, 2020
For reference, this is what you can do with
aiohttp: https://docs.aiohttp.org/en/stable/client_reference.html#aiohttp.TCPConnectorlovelydinosaur commentedon May 13, 2020
@hynek - Gotcha. The DNS caching isn't something I think we need to introduce, since it adds less reliable side-effects. Perhaps it's a feasible feature at some point, but let's treat it as out of scope for purposes here.
Out of the other stuff:
asyncio.open_connectionoptions? Are they all also available in sync and trio backends?hynek commentedon May 13, 2020
I personally need to be able to force IPv4 by setting the socket family to
AF_INETto firewall whitelisting shenanigans. No clue how common that problem is though.That could be abstracted away of course to be more use friendly.
bwelling commentedon May 21, 2020
I'm planning to look at adding source_address support to the ConnectionPool objects.
@hynek - I believe that choosing an address family would be functionally equivalent to specifying a wildcard source address in that family; that is, using
AF_INETwould be the same as using a source address of0.0.0.0. This could mean that the ConnectionPool objects internally converted a family to a source address.bwelling commentedon May 22, 2020
Adding source address support is surprisingly complicated, unfortunately. Getting the address down to the various
open_tcp_streammethods isn't a problem, but the backends are more complicated.In the sync backend, the code (currently) assumes IPv4 when opening a socket. If this restriction is kept, the only change would be to optionally call
sock.bind(), but not having IPv6 support seems bad. Usingsocket.create_connectionmight be an improvement.In the trio backend,
trio.open_tcp_streamhas no ability to accept source address. This could be changed to explicitly create a socket, optionally bind it, connect it, and wrap it in atrio.SocketStream. This would lose the ability to try multiple connections until one works (which the sync code doesn't do), and would also require doing something to figure out which address family to use.The asyncio backend looks like the easiest one;
asyncio.open_connectiontakes alocal_addrparameter.yeraydiazdiaz commentedon May 24, 2020
Hey @bwelling, thanks for looking into this.
We should think about how we'd like to expose this at the top
ConnectionPoollevel in terms of arguments:That makes sense, I think we should have both a
familyand asource_addrarguments, not sure if that's what you had in mind as well.I don't think we need to keep that restriction, seems like an oversight on our part since we tend to focus on the async side of things. I see no reason we can't use the family and/or source address in the
socketcall.I'm not familiar with Trio but I think it's probably ok to lose the happy eyeballs feature as a tradeoff for ensuring the local address constraint is met. I'm curious why Trio does not support setting local address though.
Yep, seems straightforward 🙂
BTW, there's not a lot of documentation in terms of contributing to httpcore, but generally we work on the async side and run
scripts/unasyncto generate the sync version, you may have already figured that out but wanted to make sure it's clear.bwelling commentedon May 26, 2020
I believe so. The top layer could probably convert the family into a wildcard source address, if we wanted to simplify the parameters passed to the lower layers. Similarly, the top layer could convert an address string into an address tuple.
Using
socket.create_connection()would be another alternative, I think.There's a comment in the Trio code about supporting potentially local address in the future, so I don't think it's intentional.
Getting Trio to work will not be trivial, as the current code passes the hostname to trio, which does the DNS resolution. This would need to be changed to do the DNS resolution outside of trio, so that the calling code has an address, and can open a socket in the correct address family. That is, more or less replicate a lot of the
open_tcp_streamlogic. I don't like this idea, but I don't have a better one.Yeah. I noticed that after making (small, fortunately) changes to the sync side and having
scripts/testoverwrite them.lovelydinosaur commentedon May 27, 2020
So, we don't necessarily need to have all options supported on all backends.
I think the first point here is just determining what additional arguments we think we'd want to accept.
It looks like it'd make sense for us to add arguments for
family=0andlocal_addr=Noneright?That'd mean:
family=0, local_addr=Noneto the ConnectionPool__init__and proxy__init__cases.family=0, local_addr=Noneto the backendopen_tcp_streamAPI.In the sync case
Pass
familythrough to the socket instead ofsocket.AF_INET. (Should we be defaulting tosocket.AF_INETor0?)local_addris equivalent tosource_addressin sync socket code our sync backend should add.bind(local_addr)if this is not None.In the asyncio case
Pass
familyandlocal_addrthrough to theasyncio.open_connectioncall.In the trio case
Have the backend raise an error if either argument is used, indicating that they're not currently supported. Open an issue or draft a PR with the trio project to see about getting them included in the high-level API.
Have I got that about right? Does that all seem reasonable?
hynek commentedon May 27, 2020
Wait, httpx disables IPv6 by default? Changing that is breaking but lucky you're still in 0ver yolo land. :) So yeah,
0.lovelydinosaur commentedon May 27, 2020
Appears to be an oversight in the sync case, yup...
httpcore/httpcore/_backends/sync.py
Line 133 in 221754b
Not for asyncio or trio.
lovelydinosaur commentedon May 27, 2020
Resolved in #97
njsmith commentedon May 27, 2020
Ah yeah, supporting source interfaces in
trio.open_tcp_streamis just one of those things that's been bobbing gently in the todo list for the last few years: python-trio/trio#275There are a few boring API details to figure out but it shouldn't be too hard to add.
lovelydinosaur commentedon Sep 24, 2020
Closing this off, since we haz these now.