[Shiny new list address! This is exciting!] 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? (While we're at it, IReactorTCP also claims that 'timeout' and 'bindAddress' are required parameters, even though all the implementations make them optional.)
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
On Aug 7, 2021, at 2:15 PM, Tom Most <twm@freecog.net> wrote:
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.
+1 from me on this, it seems to fall squarely into the GVOS exception even if the thing that's grossly broken here is the specification rather than the implementation :-). -g
On Aug 7, 2021, at 2:15 PM, Tom Most <twm@freecog.net> wrote:
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).
I agree with the general point about optionality, but I would say that a zillion clients would break with your implementation if you required bindAddress and timeout, so in this case let's just make IReactorTCP reflect actual reality. -g
Thanks Tom, Glyph. I raised an issue for the `host` parameter at https://twistedmatrix.com/trac/ticket/10251. Who knows, I might even get round to opening a PR for it real soon now. On 08/08/2021 03:22, Glyph wrote:
On Aug 7, 2021, at 2:15 PM, Tom Most <twm@freecog.net <mailto:twm@freecog.net>> wrote:
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).
I agree with the general point about optionality, but I would say that a zillion clients would break with your implementation if you required bindAddress and timeout, so in this case let's just make IReactorTCP reflect actual reality.
Is there a way to define a parameter as optional in an Interface, without forcing the Interface to define what the default values are? It seems reasonable to me that different implementations might want to have different defaults for these parameters, so the default values should be properties of the implementation, not the interface. R
Richard van der Hoff wrote:
On Aug 7, 2021, at 2:15 PM, Tom Most <twm@freecog.net mailto:twm@freecog.net> wrote: 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). I agree with the general point about optionality, but I would say that a zillion clients would break with your implementation if you required bindAddress and timeout, so in this case let's just make IReactorTCP reflect actual reality. Is there a way to define a parameter as optional in an Interface, without forcing the Interface to define what the default values are? It seems reasonable to me that different implementations might want to have different defaults for these parameters, so the default values should be
Thanks Tom, Glyph. I raised an issue for the `host` parameter at https://twistedmatrix.com/trac/ticket/10251. Who knows, I might even get round to opening a PR for it real soon now. On 08/08/2021 03:22, Glyph wrote: properties of the implementation, not the interface. R
I think you can use an ellipsis: `def ham(spam: Typ = ...):`
participants (4)
-
Glyph -
Richard van der Hoff -
Thomas Grainger -
Tom Most