On Jun 7, 2010, at 6:07 PM, Christopher Armstrong wrote:
But will that "lower-level than endpoints" API need to be TCP-specific?
We don't have a listenSerialPort, so why do we need listenTCP (by the way, we should totally have a SerialPortEndpoint)?
We totally *should* have a SerialPortEndpoint, but I definitely prefer the way that calling 'reactor.listenTCP(...)' implicitly selects a back-end to the platform-detection junk at the bottom of twisted.internet.serialport. The wonky platform-detection has some practical implications, as shown here: http://twistedmatrix.com/trac/ticket/3802. I would actually prefer a nice 'AttributeError: SelectReactor has no attribute "listenSerialPort"' to the mess that it currently produces.
I understand your point that some reactors provide addReader while others provide addOverlappedIOObject (or whatever the heck it's called), but an Endpoints implementation can DTRT based on the interfaces that the supplied reactor provides, so I don't see why TCPServerEndpoint can't just instantiate a port and call addReader/addWriter or the IOCPreactor equivalent.
And maybe we need a better way to deal with the differences between those reactors, but I don't see why that requires us to have public transport-specific methods on the reactor.
Well, we need to deal with the differences *somehow*. Currently, we just have a listenTCP on the reactor. This makes the implementation of endpoints dead simple, and it doesn't really vary at all per-platform. It seems to work okay; the main drawback, as far as I can tell, is that it deepens the inheritance hierarchy of the reactors a bit, because we use inheritance to grab the many similar method implementations that many similar reactors share.
In the current model, we can add new types of socket-ish things - the ones that I can think of which may one day actually matter are Java SelectableChannel objects and maybe System.Net.Socket.Select if we ever support Jython and IronPython, respectively - by adding new reactors, and these things need new reactors anyway in order to invoke the multiplexors that understand these objects. So that sorta makes sense to me. You call listenTCP on something somewhat opaque, and the opaque thing knows how to give you back an appropriate IListeningPort, ITransport, etc. With the reactor plugin API, we can even add these new types of things outside of Twisted, pretty straightforwardly.
This dispatch point *could* be handled internally in each endpoint implementation, and that does seem hypothetically more elegant to me, since it's the endpoint that should care about the TCP-ness and the reactor that should care about the file-descriptor-ness, abstractly. But at some point, the rubber must meet the road, and IReactorFDSet needs to be mapped to twisted.internet.tcp.Port, and twisted.internet.tcp.Connector, or similar, and IReactorIOCPMumbleMumble (btw, this interface should exist) to twisted.internet.iocpreactor.tcp.Port, etc.
If I want to do that mapping under the current system, I just implement my own reactor plugin and implement listenTCP to do the right thing. Under a new system where the endpoint was responsible for handling that mapping, can I add a reactor plugin that has a meaningfully different idea of what a multiplexable I/O resource is? I have some vague ideas... something sort of shaped like an adapter registry, maybe? A different plugin API, for things to talk specifically to the TCP endpoint code? I can brainstorm up some vague ideas but none of them really sit right, and certainly none of them are as straightforward as just implementing a bunch of methods specified by interfaces.
Now, I'm pretty sure you know how I feel about this, Chris, but just so nobody else takes away the wrong conclusion from this: I'm not saying that the current internal reactor factoring is perfect, or even particularly good. That code needs a lot of cleanup, a lot of documentation, and in many places a general sorting-out of what really constitutes the intended public API. It's far too hard to implement an external reactor, because you can't sensibly inherit any of that code which all reactors practically need to inherit. It's mostly undocumented and it has even changed incompatibly a few times. For example, http://twistedmatrix.com/trac/changeset/24132 incompatibly changed the signature of 'tcp.Port.__init__'. However, it seems like a more straightforward job to me to figure out a reasonable signature for tcp.Port.__init__, and to do some general de-duplication of code between the IOCP modules and the UNIX-file-descriptor modules, than to come up with a whole new way to correlate endpoint implementations to their respective concrete transport implementations.
This quality problem in the core multiplexing code is actually one of the reasons I want to vocally defend the current architecture. Until we have a defined path forward, with a clear design that's better in a way that has some positive, practical ramifications, I don't want to put anyone off doing this necessary maintenance work in twisted.internet. There's not a huge amount of maintenance going on in that area already, but if we adopt the attitude of "Oh, let's forget about that, it's going to get deprecated anyway", I have a feeling the amount of work will drop to zero.
However, of course I'm not advocating we jump the gun on deprecating listenTCP and so forth. We shouldn't deprecate them now, and when we do, we should make it an *extremely* long period of PendingDeprecation followed by Deprecation.
Well, Python 2.7 isn't going to display DeprecationWarning by default anyway, so I think PendingDeprecation may be pointless. But I definitely echo the sentiment, regardless.