python / cpython

The Python programming language
https://www.python.org
Other
63.35k stars 30.32k forks source link

Add an `proto_addr_info` argument to sock_connect in asyncio #123174

Open Aperence opened 2 months ago

Aperence commented 2 months ago

Feature or enhancement

Proposal:

Allow the selection of the protocol used by getaddrinfo by adding an additional, optional parameter proto_addr_info which defaults to None.

If not specified, use the protocol used by the socket ( = previous behaviour).

This change is needed to allow custom protocols to be used with the sock_connect, because these protocols may raise an error when calling getaddrinfo (result in a ai_proto not supported error). An example of such protocol is MPTCP (https://mptcp.dev)

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

picnixz commented 2 months ago

This requires more research than just allowing blind protocols like that. The protocol is passed to _ipaddr_info which itself checks if proto not in {0, socket.IPPROTO_TCP, socket.IPPROTO_UDP}, so it appears we are already restricting protocols. In addition, it doesn't seem right to use the same socket object but with a different protocol and it would have been better to have a socket that has the protocol we need before calling this function (and if the protocol is not supported then I don't think we need to hack this. Alternatively you could subclass the method if you want)

I would suggest discussing this first on discourse and close this issue. @kumaraditya303 what are your thoughts on this matter?

ZeroIntensity commented 2 months ago

Alternatively you could subclass the method if you want

Subclassing (or implementing your own function) is a good point -- that's a pretty easy workaround. Is there a case where you can't workaround this with your own method?

Aperence commented 2 months ago

Well I wanted to add support to enable MPTCP in aiohttp, however this method is exactly the one which is blocking for the moment (because of the ai_proto not supported), so the easiest patch was to add an extra parameter.