On Fri, Aug 6, 2021, at 1:53 AM, Richard van der Hoff wrote:
[Shiny new list address! This is exciting!]
[Yeah wow, I sure missed a lot stepping away for a weekend!]
So... IReactorTCP.connectTCP claims that 'host' should be a 'bytes': https://twistedmatrix.com/documents/current/api/twisted.internet.interfaces....
PosixReactorBase.connectTCP doesn't tell us what type it should be, but, via tcp.Connector, ends up using it to construct an IPv4Address, which reckons it should be a 'str': https://twistedmatrix.com/documents/current/api/twisted.internet.address.IPv.... Connector also uses it to construct a tcp.Client, where it gets used as part of tcp.Client.addr, which is again supposed to be a 'str'.
I think in practice, either works, but it looks like most things *expect* it to be a 'str', with grudging support for 'bytes'. So my impression is the definition of the interface is wrong. Any thoughts/comments?
Yup, the interface is wrong. It's not the only one [1] [2]. I suspect that this is a Python 3 porting error. Hostnames and IP addresses are native strings in e.g. socket.socket and in practice that seems to be what they are in Twisted. I think Names is perhaps the main exception, but then you'd kinda expect it to be lower level.
i think that we should treat this as a simple bug and correct the type. Any IReactorTCP implementation that strictly implemented the interface-as-written by _only_ accepting bytes wouldn't work, so there isn't any compatibility hazard.
Would you file a ticket?
[1]: https://twistedmatrix.com/trac/ticket/9713 [2]: https://twistedmatrix.com/trac/ticket/7302
(While we're at it, IReactorTCP also claims that 'timeout' and 'bindAddress' are required parameters, even though all the implementations make them optional.)
This one seems less clear-cut to me. I think that you could use connectTCP through and endpoint like IHostnameEndpoint and they'd always be passed.
Perhaps optional parameters are usually not a good idea for interfaces? I'm thinking of IAgent.request [3], where the optionality of the headers parameter leads to a lot of similar-looking conditional code in every implementer. Best to hoist that logic to a top-level facade (e.g., Treq).
[3]: https://twistedmatrix.com/documents/current/api/twisted.web.iweb.IAgent.html...
---Tom