endpoint: stdio

[glyph]

You should be able to run a Twisted server via a facility like inetd, or just as a subprocess for debugging.

This is a little tricky, as there can only be one, and it will interact with daemonization and other implementation details, and may need to have some nasty clever bits in order to stop the reactor when the connection is done.

I think this should be invoked as something like: serverFromString("stdio").

[itamar]

inetd is not the only use case; one might want to e.g. run a pipeline tool (assuming ticket #3442 is fixed). #3442 might actually affect the inetd case too, actually...

[glyph]

Replying to itamar:

inetd is not the only use case; one might want to e.g. run a pipeline tool

absolutely!

(assuming ticket #3442 is fixed). #3442 might actually affect the inetd case too, actually...

Actually, this is totally orthogonal. #3442 is about sharing the same file description with other processes, as with a controlling terminal. But the read end and the write end of a pipe are distinct file descriptions, so you can mess with your end without disturbing your peer's view of the socket or pipe. In the inetd case, the file descriptor is all yours (plus, in that case, it's a socket, and the other end is quite possibly in somebody else's kernel).

But even in the local case the flags are independent:

>>> from os import O_NONBLOCK, pipe
>>> from fcntl import F_GETFL, F_SETFL, fcntl
>>> pipe()
(3, 4)
>>> fcntl(3, F_GETFL)
0
>>> fcntl(4, F_GETFL)
1
>>> fcntl(3, F_SETFL, O_NONBLOCK)
0
>>> fcntl(3, F_GETFL)
4
>>> fcntl(4, F_GETFL)
1

QED.

[ashfall]

(In [34517]) Branching to 'stdio-endpoint-4697'

[ashfall]

 Build Results

[ashfall]

(In [34615]) Branching to 'stdio-endpoint-4697-2'

[exarkun]

Thanks. This looks pretty nice. Just a few minor points:

1. The stdioProtocol attribute of the endpoint doesn't look like it's needed. It's also not part of the endpoint interface, so code that works with arbitrary endpoints won't be able to use it (in other words, no other endpoints have an attribute with that name).
2. The test method docstrings should be a little more detailed about what's being tested, and not start with "Test to check". Document test methods like "The reactor passed to the endpoint is set as its _reactor attribute."
3. There should also be a test verifying that a PipeAddress instance is passed to buildProtocol. I suggest doing this with a custom factory that records the address passed to it on an attribute of itself and then using assertIsInstance in the test to verify that object was an instance of PipeAddress.
4. It looks like Windows has some difficulty with this code. It may be necessary to move the definition of PipeAddress out of _process.py, since that module is not importable on Windows (check out the build results on the Windows slaves for details).

Thanks again!

[ashfall]

Fixed. (Hopefully the build failures as well)

 Build Results

[ashfall]

I pasted build results for a wrong branch in the above comment, sorry about the same. Here's the appropriate link:  Build Results

[ashfall]

Found more errors, working on it.

[ashfall]

Looks better now. Sorry for all the mess.

 Build Results

[exarkun]

Thanks.

1. in twisted/internet/test/test_endpoints.py, stdioFactory is misnamed according to the naming convention. Classes are named like StdioFactory.
2. Also don't write "should" in a test method docstring.
3. The docstring for test_protocol talks about something not very interesting. The test isn't a test for factories. It's a test for the stdio endpoint. And in particular, it's a test for the stdio endpoint using the factory to get the protocol instance to use. So the docstring should talk about that instead. We already know that Factory.buildProtocol instantiates whatever the factory's protocol attribute is set to.
4. There are some reactor dirty warnings when the tests run on Windows. This is caused by only calling loseConnection, not waiting for the reactor to finish disconnecting the stdio connection. There are two ways to approach solving this. One would be to use a fake reactor that doesn't really establish a stdio connection. The other would be to wait for connectionLost to be called on the protocol before firing the Deferred returned to trial. If you'd rather defer this fix to another ticket, I think that's fine (just please file that ticket when you close this one).
5. The Win32PipeAddress subclass in twisted/internet/stdio.py is unnecessary. Also, I don't quite understand how test_address passes with the implementation working this way. :) You can provide a new name for an import in a couple ways. Either PipeAddress = Win32Address or by changing the import to from ... import Win32PipeAddress as PipeAddress. This avoids creating a new class, which is wrong, because the Windows stdio implementation won't actually use the new class, it'll keep using the base class.
6. There is one pyflakes warning for endpoints.py and one pyflakes warning for stdio.py

Thanks again

[ashfall]

Replying to exarkun:

1. There are some reactor dirty warnings when the tests run on Windows. This is caused by only calling loseConnection, not waiting for the reactor to finish disconnecting the stdio connection. There are two ways to approach solving this. One would be to use a fake reactor that doesn't really establish a stdio connection. The other would be to wait for connectionLost to be called on the protocol before firing the Deferred returned to trial. If you'd rather defer this fix to another ticket, I think that's fine (just please file that ticket when you close this one).

I'll file a new ticket once this is ready to merge.

[exarkun]

Thanks! A couple more tiny points:

1. There's a pyflakes warning in test_endpoints.py
2. StandardIOEndpoint.listen's docstring appears to be truncated :) Please make it a bit more specific, like the docstrings for other listen implementations.

Once those are addressed, please merge. Thanks again.

[ashfall]

Filed #5629 for the string description plugin, and #5630 for the dirty reactor warnings on Windows.

[ashfall]

(In [34653]) Merge stdio-endpoint-4697-2: An endpoint interface that listens on stdin/stdout

Author: ashfall Reviewer: exarkun Fixes: #4697

Added StandardIOEndpoint to twisted/internet/endpoints.py, along with corresponding tests in twisted/internet/test/test_endpoints.py

[ashfall]

Replying to ashfall:

Filed #5629 for the string description plugin, and #5630 for the dirty reactor warnings on Windows.

Err, I mean #5729 and #5730 respectively.