I've so far been lurking on the tulip/async discussions, as although I'm interested, I have no specific need for writing high-performance network code. However, I hit a use case today which seems to me to be ideal for an async-style approach, and yet I don't think it's covered by the current PEP. Specifically, I am looking at monitoring a subprocess.Popen object. This is basically an IO loop, but monitoring the 3 pipes to the subprocess (well, only stdout and stderr in my case...). Something like add_reader/add_writer would be fine, except for the fact that (a) they are documented as low-level not for the user, and (b) they don't work in all cases (e.g. in a select-based loop on Windows). I'd like PEP 3156 to include some support for waiting on IO from (one or more) subprocesses like this in a cross-platform way. If there's something in there to do this at the moment, that's great, but it wasn't obvious to me when I looked... Paul.
On Wed, Jan 16, 2013 at 7:12 AM, Paul Moore <p.f.moore@gmail.com> wrote:
This is a great use case. The right approach would probably be to define a new Transport (and an event loop method to create one) that wraps pipes going into and out of a subprocess. The new method would have a standard API (probably similar to that of subprocess), whereas there would be different implementations of the Transport based on platform and event loop implementation (similar to the way the subprocess module has quite different implementations). Can you check out the Tulip source code (code.google.com/p/tulip) and come up with a patch to do this? I'll gladly review it. It's fine to only cover the UNIX case for now. -- --Guido van Rossum (python.org/~guido)
On 16 January 2013 17:52, Guido van Rossum <guido@python.org> wrote:
I'll have a look. There *is* one problem, though - I imagine it will be relatively easy to put something together that works on Unix, as waiting on pipes is covered by the existing select/poll mechanisms. But I'm on Windows, so I won't be able to test it. And on Windows, there's no mechanism in place to wait on arbitrary filehandles, so the process wait mechanism is a much harder nut to crack. Chicken and egg problem... Maybe I'll start by looking at waiting on arbitrary filehandles, and use that to build the process approach. Unfortunately, I don't think IOCP is any more able to wait on arbitrary files than select - see my followup to an older thread on Richard's work there. Or maybe I'll set up a hacking environment in a Linux VM or something. That'd be a fun experience in any case. I'll have to get my brain round the existing spec as well. I'm finding it hard to understand why there are so many methods on the event loop that are specific to particular use cases (for this example, your suggested method to create the new type of Transport). My instinct says that this should *also* be a good test case for a user coming up with a new type of "event source" and wanting to plug it into the event loop. Having to add a new method to the event loop seems to imply this isn't possible. OK, off to do a lot of spec reading and then some coding. With luck, you'll be patient with dumb questions from me on the way :-) Thanks, Paul
On Wed, Jan 16, 2013 at 8:10 PM, Paul Moore <p.f.moore@gmail.com> wrote:
Dealing with subprocesses on Windows in a non-blocking way is a royal pain. As far as I know, the only option is to use named pipes and block on them using a thread pool. A few years back I wrote something that did this, see the link below. However it ain't pretty.. https://bitbucket.org/geertj/winpexpect/src/tip/lib/winpexpect.py Regards, Geert
On Wed, Jan 16, 2013 at 10:10 AM, Paul Moore <p.f.moore@gmail.com> wrote:
What does the subprocess module do on Windows? (I'm in the reverse position, although I have asked the kind IT folks at Dropbox to provide me with a Windows machine.)
I'm eagerly awaiting Richard's response. AFAIK handles on Windows *are* more general than sockets...
This is mainly so that the event loop implementation can control the Transport class. Note that it isn't enough to define different Transport classes per platform -- on a single platform there may be multiple event loop implementations (e.g. on Windows you can use Select or IOCP) and these may need different Transport implementations. SO this must really be under control of the event loop object.
If the user is okay with solving the problem only for their particular platform and event loop implementation they don't need to add anything to the event loop. But for transports that make it into the PEP, it is essential that alternate implementations (e.g. one that proxies a Twisted Reactor) be in control of the Transport construction.
OK, off to do a lot of spec reading and then some coding. With luck, you'll be patient with dumb questions from me on the way :-)
I will be! -- --Guido van Rossum (python.org/~guido)
On 16/01/2013 6:21pm, Guido van Rossum wrote:
I'm eagerly awaiting Richard's response. AFAIK handles on Windows *are* more general than sockets...
I would like to modify subprocess on Windows to use file-like objects which wrap overlapped pipe handles. Then doing async work with subprocess would become relatively straight forward, and does not really require tulip or IOCP. -- Richard
On Wed, Jan 16, 2013 at 12:45 PM, Greg Ewing <greg.ewing@canterbury.ac.nz>wrote:
Tulip on UNIX already wraps pipes (and ptys, and certain other things), since the add_reader() API takes any filedescriptor (though it makes no sense for disk files because those are always considered readable). The issue is that on other platforms (read: Windows) you have to do something completely different, and hook it up to the native (IOCP) async eventloop differently. The Transport/Protocol abstraction however would be completely appropriate in both cases though (or a slightly modified version that handles stdout/stderr separately). So, just like the subprocess module contains two completely disjoint implementations for UNIX and Windows, implementing the same API, PEP 3156 could also have a standard API for running a subprocess connected with async streams connected to stdin, stdout, stderr, backed by different implementations. -- --Guido van Rossum (python.org/~guido)
On 16 January 2013 18:21, Guido van Rossum <guido@python.org> wrote:
OK, I'm reading the PEP through now. I'm happy with the basics of the event loop, and it seems fine to me. When I reached create_transport, I had to skip ahead to the definitions of transport and protocol, as create_transport makes no sense if you don't know about those. Once I've read that, though, the whole transport/protocol mechanism seems to make reasonable sense to me. Although the host and port arguments to create_transport are clearly irrelevant to the case of a transport managing a process as a data source. So (a) I see why you say I'd need a new transport creation method, but (b) it strikes me that something more general that covered both cases (and any others that may come up later) would be better. On the other hand, given the existence of create_transport, I'm now struggling to understand why a user would ever use add_reader/add_writer rather than using a transport/protocol. And if they do have a reason to do so, why does a similar reason not apply to having an add_pipe type of method for waiting on (subprocess) pipes? In general, it still feels to me like the socket use case is being treated as "special", and other data sources and sinks (subprocesses being my use case, but I'm sure others exist) are either second-class or require a whole set of their own specialised methods, which isn't practical. As a strawman type of argument in favour of extensibility, consider a very specialist user with a hardware device that sends input via (say) a serial port. I can easily imagine that user wanting to plug his device data into the Python event loop. As this is a very specialised area, I wouldn't expect the core code to be able to help, but I would expect him to be able to write code that plugs into the standard event loop seamlessly. Ideally, I'd like to use the subprocess case as a proof that this is practical. Does that make sense? Paul.
On 17 January 2013 12:23, Paul Moore <p.f.moore@gmail.com> wrote:
Thinking about this some more. The key point is that for any event loop there can only be one "source of events" in terms of the thing that the event loop checks when there are no pending tasks. So the event loop is roughly: while True: process_ready_queue() new_events = block_on_event_source(src, timeout=N) add_to_ready_queue(new_events) add_timed_events_to_ready_queue() The source has to be a unique object, as there's an OS-level wait in there, and you can't do two of them at once. As things stand, methods like add_reader on the event loop object should really be methods on the event source object (and indeed, that's more or less what Tulip does internally). Would it not make more sense to explicitly expose the event source? This is (I guess) what the section "Choosing an Event Loop Implementation" in the PEP is about. But if the event source is a user-visible object, methods like add_reader would no longer be optional event loop methods, but rather they would be methods of the event source (but only for those event sources for which they make sense). The point here is that there's a lot of event loop machinery (ready queue, timed events, run methods) that are independent of the precise means by which you poll the OS to ask "has anything interesting happened?" Abstracting out that machinery would seem to me to make the design cleaner and more understandable. Other benefits - our hypothetical person with a serial port device can build his own event source and plug it into the event loop directly. Or someone could offer a multiplexer that combines two separate sources by running them in different threads and merging the output on a queue (that may be YAGNI, though). This is really just something to think about while I'm trying to build a Linux development environment so that I can do a Unix proof of concept. Once I get started on that, I'll think about the protocol/transport stuff. Paul
(I'm responding to two separate messages in one response.) On Thu, Jan 17, 2013 at 4:23 AM, Paul Moore <p.f.moore@gmail.com> wrote:
Whoops, I should fix the order in the PEP, or at least insert forward references.
This is why there is a TBD item suggesting to rename create_transport() to create_connection() -- this method is for creating the most common type of transport only, i.e. one that connects a client to a server given by host and port.
add_reader and friends exist for the benefit of Transport implementations. The PEP even says that not all event loops need to implement these (though on UNIXy systems it is better if they do, and I am considering removing or weakening this language. Because on UNIX pipes are just file descriptors, and work fine with select()/poll()/etc., there is no need for add_pipe() (assuming that API would take an existing pipe filedescriptor and a callback), since add_reader() will do the right thing. (Or add_writer() for the other end.)
Well, sockets are treated special because on Windows they *are* special. At least the select() system call only works for sockets. IOCP supports other types of unusual handles, but the ways to create handles you can use with it are mostly custom. Basically, if you want to write code that works both on Windows and on UNIX, you have to limit yourself to sockets. (And you shouldn't use add_reader and friends either, because that limits you to the SelectSelector, whereas if you use the transport/protocol API you will be compatible with either that or IOCPSelector.)
Yes, it does make sense, but you have to choose whether to do it on Windows or on UNIX. If you use UNIX, presumably your serial port is accessible via a file descriptor that works with select/poll/etc. -- if it doesn't, you are going to have a really hard time integrating it with the event loop, you may have to use a separate thread that talks to the device and sends the data to the event loop over a pipe or something. On Windows, I have no idea how it would work, but I presume that serial port drivers are somehow hooked up to "handles" and "waitable events" (or whatever the Microsoft terminology is -- I am about to get educated about this) and then presumably it will integrate nicely with IOCP (but not with Select). I think that for UNIX, hooking a subprocess up to a transport should be easy enough (except perhaps for the stdout/stderr distinction), and your transport should use add_reader/writer. For Windows I am not sure but you can probably crib the details from the Windows-specific code in subprocess.py in the stdlib. On Thu, Jan 17, 2013 at 6:35 AM, Paul Moore <p.f.moore@gmail.com> wrote:
Right, that's the idea.
The problem with this idea is (you may have guessed it by now :-) ... Windows. On Windows, at least when using a (at this moment purely hypothetical) IOCP-based implementation of the event loop, there will *not* be an underlying Selector object. Please track down discussion of IOCP in older posts on this list. IOCP requires you to use a different paradigm, which is supported by the separate methods sock_recv(), sock_sendall() and so on. For I/O objects that are not sockets, different methods are needed, but the idea is the same: you specify the I/O, and you get a callback when it is done. This in contrast with the UNIX selector, where you specify the file descriptor and I/O direction, and you get a callback when you can read/write without blocking. This is why the event loop has the higher-level transport/protocol-based APIs: an IOCP implementation of these creates instances of a completely different transport implementation, which however have the same interface and *meaning* as the UNIX transports (e.g. the transport created by create_connection() connects to a host and port over TCP/IP and calls the protocol's connection_made(), data_received(), connection_lost() methods). So if you want a transport that encapsulates a subprocess (instead of a TCP/IP connection), and you want to support both UNIX and Windows, you have to provide (at least) two separate implementations: one on UNIX that uses add_reader() and friends, and one on Windows that uses (I don't know what, but something). Each of these implementations by itself is dependent on the platform (and the specific event loop implementation); but together they cover all supported platforms. If you develop this as 3rd party code, and you want your users not to have to write platform-specific code, you have to write a "start subprocess" function that inspects the platform (and the event loop implementation) and then imports and instantiates the right transport implementation for the platform. If we want to add this to the PEP, the right thing is to add a "start subprocess" method to the event loop API (which can be identical to the start subprocess function in your 3rd party package :-).
It is abstracted out in the implementation, but I hope I have explained with sufficient clarity why it should not be abstracted out in the PEP: the Selector abstraction only works on UNIX (or with sockets on Windows). Also note a subtlety in the PEP: while it describes a platform-independent API, it doesn't preclude that some parts of that API may have platform-specific behaviors -- for example, add_reader() may only take sockets on Windows (and in Jython, I suspect, where select() only works with sockets), but takes other file descriptors on UNIX, so you can implement your own subprocess transport for UNIX. Similarly, the PEP describes the interface between transports and protocols, but does not give you a way to construct a transport except for TCP/IP connections. But the abstraction is usable for other purposes too, and this is intentional! (E.g. you may be able to create a transport that uses a subprocess running ssh to talk to a remote server, which might be used to "tunnel" HTTP, so it would make sense to connect this custom transport with a standard HTTP protocol implementation.)
Other benefits - our hypothetical person with a serial port device can build his own event source and plug it into the event loop directly.
I think I've answered that above.
I think there are Twisted reactor implementations that do things like this. My hope is that a proxy between the Twisted reactor and the PEP 3156 interface will enable this too -- and the event loop APIs for working with transports and protocols are essential for this purpose. (Twisted has a working IOCP reactor, FWIW.)
I think it would be tremendously helpful if you tried to implement the UNIX version of the subprocess transport. (Note that AFAIK Twisted has one of these too, maybe you can get some implementation ideas from them.) -- --Guido van Rossum (python.org/~guido)
Hmm, there may still be something to the idea of clearly separating out "for everyone" and "for transports" methods. Even if that's just a split in the documentation, similar to the "for everyone" vs "for the executor" split in the concurrent.futures implementation. -- Sent from my phone, thus the relative brevity :)
On 17 January 2013 19:10, Guido van Rossum <guido@python.org> wrote:
You were right. In starting to do so, I found out that my thinking has been solely based on a callback style of programming (users implement protocol classes and code the relevant "data received" methods themselves). From looking at some of the sample code, I see that this is not really the intended usage style. At this point my head exploded. Coroutines, what fun! I am now reading the sample code, the section of the PEP on coroutines, and the mailing list threads on the matter. I may be some time :-) (The technicalities of the implementation aren't hard - it's just a data_received type of protocol wrapper round a couple of pipes. It's the usability and design issues that matter, and they are strongly affected by "intended usage"). Paul PS From the PEP, it seems that a protocol must implement the 4 methods connection_made, data_received, eof_received and connection_lost. For a process, which has 2 output streams involved, a single data_received method isn't enough. I see two options - having 2 separate protocol classes involved, or having a process protocol with a different interface. Neither option seems obviously best, although Twisted appears to use different protocol types for different types of transport. How critical is the principle that there is a single type of protocol to the PEP?
On Thu, Jan 17, 2013 at 3:44 PM, Paul Moore <p.f.moore@gmail.com> wrote:
Right, this is a very good observation.
Not critical at all. The plan for UDP (datagrams in general) is to have different protocol methods as well. TBH I would be happy with a first cut that only deals with stdout, like os.popen(). :-) Note that I am intrigued by this problem as well and may be hacking up a version for myself in my spare time. -- --Guido van Rossum (python.org/~guido)
Paul Moore wrote:
It looks like there would have to be at least two Transport instances involved, one for stdin/stdout and one for stderr. Connecting them both to a single Protocol object doesn't seem to be possible with the framework as defined. You would have to use a couple of adapter objects to translate the data_received calls into calls on different methods of another object. This sort of thing would be easier if, instead of the Transport calling a predefined method of the Protocol, the Protocol installed a callback into the Transport. Then a Protocol designed for dealing with subprocesses could hook different methods of itself into a pair of Transports. Stepping back a bit, I must say that from the coroutine viewpoint, the Protocol/Transport stuff just seems to get in the way. If I were writing coroutine-based code to deal with a subprocess, I would want to be able to write coroutines like def handle_output(stdout): while 1: line = yield from stdout.readline() if not line: break mungulate_line(line) def handle_errors(stderr): while 1: line = yield from stderr.readline() if not line: break complain_to_user(line) In other words, I don't want Transports or Protocols or any of that cruft, I just want a simple pair of async stream objects that I can read and write using yield-from calls. There doesn't seem to be anything like that specified in PEP 3156. It does mention something about implementing a streaming buffer on top of a Transport, but in a way that makes it sound like a suggested recipe rather than something to be provided by the library. Also it seems like a lot of layers of overhead to go through. On the whole, in PEP 3156 the idea of providing callback-based interfaces with yield-from-based ones built on top has been pushed way further up the stack than I imagined it would. I don't want to be *forced* to write my coroutine code at the level of Protocols; I want to be able to work at a lower level than that. -- Greg
On Thu, Jan 17, 2013 at 11:17 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
So far this makes sense. But for this specific case there's a simpler solution -- require the protocol to support a few extra methods, in particular, err_data_received() and err_eof_received(), which are to stderr what data_received() and eof_received() are for stdout. (After all, the point of a subprocess is that "normal" data goes to stdout.) There's only one input stream to the subprocess, so there's no ambiguity for write(), and neither is there a need for multiple connection_made()/lost() methods. (However, we could argue endlessly over whether connection_lost() should be called when the subprocess exits, or when the other side of all three pipes is closed. :-)
Hm. Not excited. I like everyone using the same names for these callback methods, so that a reader (who is familiar with the transport/protocol API) can instantly know what kind of callback it is and what its arguments are. (But see Nick's simple solution for having your cake and eating it, too.)
This is a good observation -- one that I've made myself as well. I also have a plan for dealing with it -- but I haven't coded it up properly yet and consequently I haven't written it up for the PEP yet either. The idea is that there will be some even-higher-level functions for tasks to call to open connections (etc.) which just give you two unidrectional streams (one for reading, one for writing). The write-stream can just be the transport (its write() and writelines() methods are familiar from regular I/O streams) and the read-stream can be a StreamReader -- a class I've written but which needs to be moved into a better place: http://code.google.com/p/tulip/source/browse/tulip/http_client.py#37 Anyway, the reason for having the transport/protocol abstractions in the middle is so that other frameworks can ignore coroutines if they want to -- all they have to do is work with Futures, which can be fully controlled through callbacks (which are native at the lowest level of almost all frameworks, including Tulip / PEP 3156).
It'll be in the stdlib, no worries. I don't expect the overhead to be a problem.
You can write an alternative framework using coroutines and callbacks, bypassing transports and protocols. (You'll still need Futures.) However you'd be missing the interoperability offered by the protocol/transport abstractions: in an IOCP world you'd have to interact with the event loop's callbacks differently than in a select/poll/etc. world. PEP 3156 is trying to make different groups happy: people who like callbacks, people who like coroutines; people who like UNIX, people who like Windows. Everybody may have to compromise a little bit, but the reward will (hopefully) be better portability and better interoperability. -- --Guido van Rossum (python.org/~guido)
On 18 January 2013 22:15, Guido van Rossum <guido@python.org> wrote:
While I don't really care about arguing over *when* connection_lost should be called, it *is* relevant to my thinking that getting notified when the process exits doesn't seem to me to be possible - again it's the issue that the transport can't ask the event loop to poll for anything that the event loop isn't already coded to check. So (once again, unless I've missed something) the only viable option for a standalone transport is to call connection_lost when all the pipes are closed. Am I still missing something? Paul
On Fri, Jan 18, 2013 at 2:57 PM, Paul Moore <p.f.moore@gmail.com> wrote:
That is typically how these things are done (e.g. popen and subprocess work this way). It is also probably the most useful, since it is *possible* that the parent process forks a child and then exits itself, where the child does all the work of the pipeline.
Am I still missing something?
I believe it is, at least in theory, possible to implement waiting for the process to exit, using signals. The event loop can add signal handlers, and there is a signal that gets sent upon child process exit. There are lots of problems here (what if some other piece of code forked that process) but we could come up with reasonable solutions for these. However waiting for the pipes closing makes the most sense, so no need to bother. :-) -- --Guido van Rossum (python.org/~guido)
On 18Jan2013 15:01, Guido van Rossum <guido@python.org> wrote: | It is also probably the most useful, since it is | *possible* that the parent process forks a child and then exits | itself, where the child does all the work of the pipeline. For me, even common. I often make grandchildren instead of children when only the I/O matters so that I don't leave zombies around, nor spurious processes to interfere with wait calls. -- Cameron Simpson <cs@zip.com.au> To have no errors Would be life without meaning No struggle, no joy - Haiku Error Messages http://www.salonmagazine.com/21st/chal/1998/02/10chal2.html
Guido van Rossum wrote:
You don't seem to follow this philosophy anywhere else in the PEP, though. In all the other places a callback is specified, you get to pass in an arbitrary function. The PEP offers no rationale as to why transports should be the odd one out.
I was hoping there would be a slightly higher-level layer, that provides a coroutine interface but hides the platform differences. What would you think of the idea of making the Transport objects themselves fill both roles, by having read_async and write_async methods? They wouldn't have to do any buffering, I'd be happy to wrap another object around it if I wanted that. -- Greg
On Fri, Jan 18, 2013 at 4:42 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Well, yes, it *is* the odd one (or two, counting start_serving()) out. That's because it is the high-level API.
Hm, Transports+Protocols *is* the higher level layer.
You could code that up very simply using sock_recv() and sock_sendall(). But everyone who's thought about performance of select/poll/etc., seems to think that that is not a good model because it will cause many extra calls to add/remove reader/writer. -- --Guido van Rossum (python.org/~guido)
On Fri, Jan 18, 2013 at 5:15 PM, Guido van Rossum <guido@python.org> wrote:
Using separate methods for stderr breaks compatibility with existing Protocols for no good reason (UDP needs a different protocol interface because individual datagrams can't be concatenated; that doesn't apply here since pipes are stream-oriented). We'll have intermediate Protocol classes like LineReceiver that work with sockets; why should they be reimplemented for stderr? It's also likely that if I do care about both stdout and stderr, I'm going to take stdout as a blob and redirect it to a file, but I'll want to read stderr with a line-oriented protocol to get error messages, so I don't think we want to favor stdout over stderr in the interface. I think we should have a pipe-based Transport and the subprocess should just contain several of these transports (depending on which fds the caller cares about; in my experience I rarely have more than one pipe per subprocess, but whether that pipe is stdout or stderr varies). The process object itself should also be able to run a callback when the child exits; waiting for the standard streams to close is sufficient in most cases but not always. -Ben
On Mon, Jan 21, 2013 at 1:23 PM, Ben Darnell <ben@bendarnell.com> wrote:
This is a good point.
That all depends rather on the application.
Unfortunately you'll also need a separate protocol for each transport, since the transport calls methods with fixed names on the protocol (and you've just argued that that we should stick to that -- and I agree :-). Note that since there's (normally) only one input file to the subprocess, only one of these transports should have a write() method -- but both of them have to call data_received() and potentially eof_received() on different objects. And in this case it doesn't seem easy to use the StreamReader class, since you can't know which of the two (stdout or stderr) will have data available first, and guessing wrong might cause a deadlock. (So, yes, this is a case where coroutines are less convenient than callbacks.) -- --Guido van Rossum (python.org/~guido)
On Mon, Jan 21, 2013 at 9:31 PM, Guido van Rossum <guido@python.org> wrote:
Exactly.
Well, to be precise I was arguing that pipe transports should work the same way as socket transports. I'm still not a fan of the use of fixed method names. (As an alternative, what if protocols were just callables that took a Future argument? for data_received future.result() would return the data and for eof_received and connection_lost it would raise an appropriate exception type. That just leaves connection_made, which I was arguing in the other thread should be on the protocol factory instead of the protocol.)
I'd actually give stdin its own transport and protocol, distinct from stdout and stderr (remember that using all three pipes on the same process is relatively uncommon). It's a degenerate case since it will never call data_received, but it's analogous to the way that subprocess uses three read-only and write-only file objects instead of trying to glue stdin and stdout together. This is fairly new and little-tested, but it shows the interface I have in mind: http://tornado.readthedocs.org/en/latest/process.html#tornado.process.Subpro...
I'm not sure I follow. Couldn't you just attach a StreamReader to each stream and use as_completed to read from them both in parallel? You'd get in trouble if one of the streams has a line longer than the StreamReader's buffer size, but that sort of peril is everywhere if you're using both stdout and stderr, no matter what the interface is (unless you just use a large or unlimited buffer and hope you won't run out of memory, like subprocess.communicate). At least with "yield from stderr_stream.readline()" you're better off than with a synchronous subprocess since the StreamReader's buffer size is adjustable, unlike the pipe buffer size. -Ben
-- --Guido van Rossum (python.org/~guido)
On Thu, Jan 17, 2013 at 2:40 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Shouldn't this be called create_internet_transport or something like that?
I just renamed it to create_connection(), like I've been promising for a long time. -- --Guido van Rossum (python.org/~guido)
On Thu, Jan 17, 2013 at 8:59 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Basically yes, in this context. The same assumption underlies socket.getaddrinfo() in the stdlib. If you have a CORBA system lying around and you want to support it, you're welcome to create the transport connection function create_corba_connection(). :-) -- --Guido van Rossum (python.org/~guido)
On Fri, Jan 18, 2013 at 6:08 PM, Paul Moore <p.f.moore@gmail.com> wrote:
I'm not sure why CORBA would be a transport in its own right rather than a protocol running over a standard socket transport. Transports are about the communications channel - network sockets - OS pipes - shared memory - CANbus - protocol tunneling Transports should only be platform specific at the base layer where they actually need to interact with the OS through the event loop. Higher level transports should be connected to lower level protocols based on APIs provided by those transports and protocols themselves. The *whole point* of the protocol vs transport model is to allow you to write adaptive stacks. To use the example from PEP 3153, to implement full JSON-RPC support over both sockets and a HTTP-tunnel you need the following implemented: - TCP socket transport - HTTP protocol - HTTP-based transport - JSON-RPC protocol Because the transport API is standardised, the JSON-RPC protocol can be written once and run over HTTP using the full stack as shown, *or* directly over TCP by stripping out the two middle layers. The *only* layer that the event loop needs to concern itself with is the base transport layer - it doesn't care how many layers of protocols or protocol-as-transport adapters you stack on top. The other thing that may not have been emphasised sufficiently is that the *protocol* APIs is completely dependent on the protocol involved. The API of a pipe protocol is not that of HTTP or CORBA or JSON-RPC or XML-RPC. That's why tunneling, as in the example above, requires a protocol-specific adapter to translate from the protocol API back to the standard transport API. So, for example, Greg's request for the ability to pass callbacks rather than needing particular method names can be satisfied by writing a simple callback protocol: class CallbackProtocol: """Invoke arbitrary callbacks in response to transport events""" def __init__(self, on_data, on_conn, on_loss, on_eof): self.on_data = on_data self.on_conn = on_conn self.on_loss = on_loss self.on_eof = on_eof def connection_made(transport): self.on_conn(transport) def data_received(data): self.on_data(data) def eof_received(): self.on_eof() def connection_lost(exc): self.on_loss(exc) Similarly, his request for a IOStreamProtocol would likely look a lot like an asynchronous version of the existing IO stack API (to handle encoding, buffering, etc), with the lowest layer being built on the transport API rather than the file API (as it is in the io module). You would then be able to treat *any* transport, whether it's an SSH tunnel, an ordinary socket connection or a pipe to a subprocess as a non-seekable stream. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 18 January 2013 08:38, Nick Coghlan <ncoghlan@gmail.com> wrote:
Interesting. On that basis, the whole subprocess interaction scenario is not a low level transport at all (contrary to what I understood from Guido's suggestion of an event loop method) and so should be built in user code (OK, probably as a standard library helper, but definitely not as specialist methods on the event loop) layered on the low-level pipe transport. That was my original instinct, but it fell afoul of 1. The Windows implementation of a low level pipe transport doesn't exist (yet) and I don't know enough about IOCP to write it [1]. 2. I don't understand the programming model well enough to understand how to write a transport/protocol layer (coroutine head explosion issue). I have now (finally!) got Guido's point that implementing a process protocol will give me a good insight into how this stuff is meant to work. I'm still struggling to understand why he thinks it needs a dedicated method on the event loop, rather than being a higher-level layer like you're suggesting, but I'm at least starting to understand what questions to ask. Paul [1] There is some stuff in the IOCP documentation about handles having to be opened in OVERLAPPED mode, which worries me here as it may imply that arbitrary pipes (such as the ones subprocess.Popen uses) can't be plugged in. It's a bit like setting a filehandle to nonblocking in Unix, but it has to be done at open time, IIUC. I think I saw an email about this that I need to hunt out.
On Fri, Jan 18, 2013 at 7:33 PM, Paul Moore <p.f.moore@gmail.com> wrote:
The creation of the pipe transport needs to be on the event loop, precisely because of cross-platform differences when it comes to Windows. On *nix, on the other hand, the pipe transport should look an awful lot like the socket transport and thus be able to use the existing file descriptor based interfaces on the event loop. The protocol part is then about adapting the transport API to coroutine friendly readlines/writelines API (the part that Guido points out needs more detail in http://www.python.org/dev/peps/pep-3156/#coroutines-and-protocols) As a rough untested sketch (the buffering here could likely be a lot smarter): # Remember we're not using preemptive threading, so we don't need locking for thread safety # Note that the protocol isn't designed to support reconnection - a new connection means # a new protocol instance. The create_* APIs on the event loop accept a protocol factory # specifically in order to encourage this approach class SimpleStreamingProtocol: def __init__(self): self._transport = None self._data = bytearray() self._pending = None def connection_made(self, transport): self._transport = transport def connection_lost(self, exc): self._transport = None # Could also store the exc directly on the protocol and raise # it in subsequent write calls if self._pending is not None: self._pending.set_exception(exc) def received_eof(self): self.transport = None if self._pending is not None: self._pending.set_result(False) def received_data(self, data): self.data.extend(data) if self._pending is not None: self._pending.set_result(True) # The writing side is fairly easy, as we just pass it through to the transport # These are all defined by PEP 3156 as non-blocking calls def write(self, data): if self._transport is None: raise RuntimeError("Connection not open") self._transport.write(data) def writelines(self, iterable): if self._transport is None: raise RuntimeError("Connection not open") self._transport.writelines(iterable) def close(self): if self._transport is not None: self._transport.close() self._transport = None def _read_from_buffer(self): data = bytes(self._data) self._data.clear() return data # The reading side has to adapt between coroutines and callbacks @coroutine def read(self): if self._transport is None: raise RuntimeError("Connection not open") if self._pending is not None: raise RuntimeError("Concurrent reads not permitted") # First check if we already have data waiting data = self._read_from_buffer() if data: return data # Otherwise wait for data # This method can easily be updated to use a loop and multiple # futures in order to support a "minimum read" parameter f = self._pending = tulip.Future() finished = yield from f data = b'' if finished else self._read_from_buffer() return data # This uses async iteration as described at [1] # We yield coroutines, which must then be invoked with yield from def readlines(self): cached_lines = self._data.split(b'\n') self._data.clear() if cached_lines[-1]: # Last line is incomplete self._data.extend(cached_lines[-1]) del cached_lines[-1] while not finished: # When we already have the data, a simple future will do for line in cached_lines: f = tulip.Future() f.set_result(line) yield f # Otherwise, we hand control to the event loop @coroutine def wait_for_line(): nonlocal finished data = yield from self.read() if not data: finished = True return b'' lines = data.split(b'\n') if lines[-1]: # Last line is incomplete self._data.extend(lines[-1]) cached_lines.extend(lines[1:-1]) return lines[0] yield wait_for_line() # Used as: pipe, stream = event_loop.create_pipe(SimpleStreamingProtocol) # Or even as: conn, stream = event_loop.create_connection(SimpleStreamingProtocol, ... # connection details) # Reading from the stream in a coroutine for f in stream.readlines(): line = yield from f [1] http://python-notes.boredomandlaziness.org/en/latest/pep_ideas/async_program... Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Fri, Jan 18, 2013 at 3:55 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Thanks for clarifying that -- I'm behind on this thread!
I have a more-or-less working but probably incomplete version checked into the tulip repo: http://code.google.com/p/tulip/source/browse/tulip/subprocess_transport.py Note that this completely ignores stderr -- this makes the code simpler while still useful (there's plenty of useful stuff you can do without reading stderr), and avoids the questions Greg Ewing brought up about needing two transports (one for stdout, another for stderr). -- --Guido van Rossum (python.org/~guido)
On 18 January 2013 22:22, Guido van Rossum <guido@python.org> wrote:
Ha! You beat me to it. OK, looking at your code, I see that you freely used the add_reader/add_writer functions and friends, and the fact that the Unix selectors handle pipes as well as sockets. With the freedom to do that, your code looks both reasonable and pretty straightforward. I was having trouble getting past the fact that this approach wouldn't work on Windows, and confusing "nonportable" with "not allowed". My apologies. You kept telling me that writing the code for Unix would be helpful, but I kept thinking in terms of writing code that worked on Unix but with portability to Windows in mind, which completely misses the point. I knew that the transport/protocol code I'd end up writing would look something like this, but TBH I'd not seen that as the interesting part of the problem... BTW, to avoid duplication of the fork/exec stuff, I would probably have written the transport to take a subprocess.Popen object as its only argument, then hooked up self._wstdin to popen.stdin and self._rstdout to popen.stdout. That requires the user to have created the Popen object with those file descriptors as pipes (I don't know if it's possible to introspect a Popen object to check that) but avoids duplicating the subprocess logic. I can probably fairly quickly modify your code to demonstrate, but it's late and I don't want to start booting my Unix environment now, so it'll have to wait till tomorrow :-) Paul.
On Fri, Jan 18, 2013 at 2:48 PM, Paul Moore <p.f.moore@gmail.com> wrote:
Glad you've got it now!
I would love for you to create that version. I only checked it in so I could point to it -- I am not happy with either the implementation, the API spec, or the unit test... -- --Guido van Rossum (python.org/~guido)
On 18 January 2013 22:53, Guido van Rossum <guido@python.org> wrote:
May be a few days before I can get to it. Apparently when Ubuntu installs an automatic upgrade, it feels that it's OK to break the wireless drivers. I now have the choice of scouring the internet on another PC to find possible solutions (so far that approach is a waste of time...), or reinstalling the OS. How do you Linux users put up with this sort of thing? :-) Seriously, I'm probably going to have to build a VM so I don't get this sort of unnecessary hardware issue holding me up. Paul
On 19 January 2013 12:12, Paul Moore <p.f.moore@gmail.com> wrote:
OK, I finally have a working VM. The subprocess test code assumes that it can call transport.write_eof() in the protocol connection_made() function. I'm not sure if that is fundamental, or just an artifact of the current implementation. Certainly if you have a stdin pipe open, you likely want to close it to avoid deadlocks, but with the subprocess.Popen approach, it's entirely possible to not open a pipe to stdin. In that case, writing to stdin is neither possible nor necessary. Clearly, writing data to stdin if you didn't open a pipe should be flagged as an error. And my immediate thought is that write_eof should also be an error. But I can imagine people wanting to write reusable protocols that pre-emptively write EOF to the stdin pipe to avoid deadlocks. So, a question: If the user passed a popen object without a stdin pipe, should write_eof be an error or should it just silently do nothing? Paul
On Tue, Jan 22, 2013 at 10:03 PM, Paul Moore <p.f.moore@gmail.com> wrote:
It should be an error. The analogy is similar to calling flush() vs close(). Calling flush() on an already closed file is an error, while you can call close() as many times as you like. If you want to ensure a pipe is closed gracefully, call close(), not write_eof(). (abort() is the method for abrupt closure). Also, I agree with the comment someone else made that attempting to pair stdin with either stderr or stdout is a bad idea - better to treat them as three independent transports (as the subprocess module does), so that the close() semantics and error handling are clear. sockets are different, as those actually *are* bidirectional data streams, whereas pipes are unidirectional. I don't know whether it's worth defining separate SimplexTransmit (e.g. stdin pipe in parent process, stdout, stderr pipes in child process), SimplexReceive (stdout, stderr pipes in parent process, stdin pip in child process), HalfDuplex (e.g. some radio transmitters) and FullDuplex (e.g. sockets) transport abstractions - I guess if Twisted haven't needed them, it probably isn't worth bothering. It's also fairly obvious how to implement the first three based on the full duplex API currently described in the PEP just be raising the appropriate exceptions. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 22 January 2013 13:34, Nick Coghlan <ncoghlan@gmail.com> wrote:
That was my original feeling - although I made my case badly by arguing in terms of portability rather than clearer design. But Guido argued for a higher-level portable subprocess transport that was implemented "under the hood" using the existing nonportable add_reader/add_writer methods on Unix, and an as-yet-unimplemented IOCP-based alternative on Windows. I still feel that a more general approach would be to have two methods on the event loop connect_input_pipe(protocol_factory, readable_pipe) and connect_output_pipe(protocol_factory, writeable_pipe) which use the standard transport/protocol methods as defined in the PEP. Then the subprocess transport can be layered on top of that as one possible example of a "higher layer" convenience transport. I know that twisted has a create_process event loop (reactor) method, but I suspect part of the reason for that is that it predates the subprocess module's unified interface. I'll try implementing the pipe transport approach and see how it looks in contrast. Paul.
On 22 January 2013 15:43, Paul Moore <p.f.moore@gmail.com> wrote:
I'll try implementing the pipe transport approach and see how it looks in contrast.
Here's a quick proof of concept (for a read pipe): class UnixEventLoop(events.EventLoop): ... @tasks.task def connect_read_pipe(self, protocol_factory, rpipe): protocol = protocol_factory() waiter = futures.Future() transport = _UnixReadPipeTransport(self, rpipe, protocol, waiter) yield from waiter return transport, protocol class _UnixReadPipeTransport(transports.Transport): def __init__(self, event_loop, rpipe, protocol, waiter=None): self._event_loop = event_loop self._pipe = rpipe.fileno() self._protocol = protocol self._buffer = [] self._event_loop.add_reader(self._pipe.fileno(), self._read_ready) self._event_loop.call_soon(self._protocol.connection_made, self) if waiter is not None: self._event_loop.call_soon(waiter.set_result, None) def _read_ready(self): try: data = os.read(self._pipe, 16*1024) except BlockingIOError: return if data: self._event_loop.call_soon(self._protocol.data_received, data) else: self._event_loop.remove_reader(self._pipe) self._event_loop.call_soon(self._protocol.eof_received) Using this to re-implement the subprocess test looks something like this (the protocol is unchanged from the existing test): def testUnixSubprocessWithPipe(self): proc = subprocess.Popen(['/bin/ls', '-lR'], stdout=subprocess.PIPE) t, p = yield from self.event_loop.connect_read_pipe(MyProto, proc.stdout) self.event_loop.run() To be honest, this looks sufficiently straightforward that I don't see the benefit in a less-general high-level transport type... Paul
I am not actually very committed to a particular design for a subprocess transport. I'll happily leave it up to others to come up with a design and make it work on multiple platforms. --Guido van Rossum (sent from Android phone)
On 22 January 2013 16:33, Guido van Rossum <guido@python.org> wrote:
OK. I've written a pipe transport (event_loop.connect_read_pipe and event_loop.connect_write_pipe) and modified the existing subprocess test to use it. I've also added a small read/write test. The code is in my bitbucket repository at https://bitbucket.org/pmoore/tulip. I'm not very happy with the call-back based style of the read/write test. I'm sure it would be much better written in an async style, but I don't know how to do so. If anyone who understands the async style better than I do can offer a translation, I'd be very grateful - I'd like to see if the resulting code looks sufficiently clear. Here's the relevant code. The biggest ugliness is the need for the two protocol classes, which basically do nothing but (1) collect data received and (2) ignore unwanted callbacks. class DummyProto(protocols.Protocol): def __init__(self): pass def connection_made(self): pass def data_received(self, data): pass def eof_received(self): pass def connection_lost(): pass class MyCollector(protocols.Protocol): def __init__(self): self.data = [] def connection_made(self): pass def data_received(self, data): self.data.append(data) def eof_received(self): pass def connection_lost(): pass def get_data(self): return b''.join(self.data) def testReadWrite(self): proc = Popen(['/bin/tr', 'a-z', 'A-Z'], stdin=PIPE, stdout=PIPE) rt, rp = yield from self.event_loop.connect_read_pipe(MyCollector, proc.stdout) wt, wp = yield from self.event_loop.connect_read_pipe(DummyProto, proc.stdin) def send_data(): wt.write("hello, world") wt.write_eof() self.event_loop.call_soon(send_data) self.event_loop.run() self.assertEqual(rp.get_data(), b'HELLO, WORLD') Paul
Guido van Rossum wrote: […]
Although 3 pipes to a subprocess (stdin, stdout, stderr) is the usual convention, it's not the only possibility, so that configuration shouldn't be hard-coded. On POSIX some programs can and do make use of the ability to have more pipes to a subprocess; e.g. the various *fd options of gnupg (--status-fd, --logger-fd, --command-fd, and so on). And some programs give the child process file descriptors that aren't pipes, like sockets (e.g. an inetd-like server that accepts a socket then spawns a subprocess to serve it). So I hope tulip will support these possibilities (although obviously the stdin/out/err style should be the convenient default). You will be unsurprised to hear that Twisted does :) (Please forgive me if this was already pointed out. It's hard keeping up with python-ideas.) -Andrew.
On 20 January 2013 22:53, Andrew Bennetts <andrew@bemusement.org> wrote:
My plan is to modify Guido's current code to take a subprocess.Popen object when creating a connection to a subprocess. So you'd use the existing API to start the process, and then tulip to interact with it. Having said that, I have no idea if or how subprocess.Popen would support the extra fds you are talking about. If you can show me some sample code, I can see what would be needed to handle it. But as far as I know, subprocess.Popen objects only have the 3 standard handles exposed as attributes - stdin, stdout and stderr. If you have to create your own pipes and manage them yourself in "normal" code, then I would expect that you'd have to do the same with tulip. That may indicate a need for (yet another) event loop API to create a pipe which can then be used with subprocess. Or you could use the add_reader/add_writer interfaces, at the expense of portability. Paul PS The above is still my plan. But at the moment, every PC in my house seems to have decided to stop working, so I'm rebuilding PCs rather than doing anything useful :-( Normal service will be resumed in due course...
On Sun, Jan 20, 2013 at 3:25 PM, Paul Moore <p.f.moore@gmail.com> wrote:
subprocess.Popen has the pass_fds argument, documented as follows: *pass_fds* is an optional sequence of file descriptors to keep open between the parent and child. Providing any *pass_fds* forces *close_fds*to be True <http://docs.python.org/dev/library/constants.html#True>. (Unix only) Eli
On 20 January 2013 23:29, Eli Bendersky <eliben@gmail.com> wrote:
I thought that was the case, but it seems like this is only really enabling you to manually manage the extra pipes as I was suggesting in my comment. My current expectation is that the API would be something like eventloop.connect_process(protocol_factory, popen_obj) and the protocol would have data_received and err_received methods called when the stdout or stderr fds have data, and the transport would have a write method to write to stdin. If anyone has a suggestion for an API that could be used for arbitrary FDs (which I presume could be either input or output) on top of this, I'd be happy to incorporate it - but personally, I can't think of anything that wouldn't be unusably complex :-( Paul
On Sun, Jan 20, 2013 at 2:53 PM, Andrew Bennetts <andrew@bemusement.org> wrote:
Hm. I agree that something to represent an arbitrary pipe or pair of pipes may be useful occasionally, and we need to have an implementation that can deal with stdout and stderr separately anyway, but I don't think such extended configurations are common enough that we need to completely generalize the API. I think it is fine to follow the example of subprocess.py, which allows but does not encourage extra pipes and treats stdin, stdout and stderr differently.
-- --Guido van Rossum (python.org/~guido)
On 18 January 2013 09:33, Paul Moore <p.f.moore@gmail.com> wrote:
Hmm, I'm looking at a pipe transport on Unix, and I find I don't know enough about programming Unix. How do I set a file descriptor (specifically a pipe) in Unix to be nonblocking? For a socket, sock.setblocking(False) does the job. But for a pipe/file, the only thing I can see is the O_NONBLOCK flag to os.open/os.pipe2. Is it not possible to set an already open file descriptor to be nonblocking? If that's the case, it means that Unix has the same problem as I suspect exists for Windows - existing pipes and filehandles can't be used in async code as they won't necessarily be in nonblocking mode. Is there a way round this on Unix that I'm not aware of? Otherwise, it seems that there's going to have to be a whole load of duplication in the "async world" (an async version of subprocess.Popen, for a start, as well as any other "open" type of calls that might need to produce handles that can be used asynchronously). Either that or everything that returns a pipe/handle that you might want to use in async code will have to grow some sort of "async" flag. Paul
On Fri, Jan 18, 2013 at 09:01:32PM +0000, Paul Moore <p.f.moore@gmail.com> wrote:
http://linuxmanpages.com/man2/fcntl.2.php The file status flags A file descriptor has certain associated flags, initialized by open(2) and possibly modified by fcntl(2). The flags are shared between copies (made with dup(2), fork(2), etc.) of the same file descriptor. The flags and their semantics are described in open(2). F_GETFL Read the file descriptor's flags. F_SETFL Set the file status flags part of the descriptor's flags to the value specified by arg. Remaining bits (access mode, file creation flags) in arg are ignored. On Linux this command can only change the O_APPEND, O_NONBLOCK, O_ASYNC, and O_DIRECT flags. Oleg. -- Oleg Broytman http://phdru.name/ phd@phdru.name Programmers don't die, they just GOSUB without RETURN.
On Sat, Jan 19, 2013 at 01:25:31AM +0400, Oleg Broytman <phd@phdru.name> wrote:
So you have to call fnctl() on the pipe's descriptor to F_GETFL flags, set O_NONBLOCK and call fnctl() to F_SETFL the new flags back. Oleg. -- Oleg Broytman http://phdru.name/ phd@phdru.name Programmers don't die, they just GOSUB without RETURN.
Paul Moore wrote:
No, it doesn't -- a fd doesn't *have* to be non-blocking in order to use it with select/poll/whatever. Sometimes people do, but only to allow a performance optimisation by attempting another read before going back to the event loop, just in case more data came in while you were processing the first lot. But doing that is entirely optional. Having said that, fcntl() is usually the way to change the O_NONBLOCK flag of an already-opened fd, althought the details may vary from one unix to another. -- Greg
On Fri, Jan 18, 2013 at 12:38 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
I don't know -- but I could imagine that a particular CORBA implementation might be provided as a set of API function calls rather than something that hooks into sockets. I don't care about CORBA, but that was the use case I intended to highlight -- something that (for whatever reason, no matter how misguided) doesn't use sockets and doesn't have an underlying file descriptor you can wait on. (IIRC most GUI frameworks also fall into that category.)
Hm. I think of transports more as an abstraction of a specific set of semantics for a communication channel -- bidrectional streams, in particular, presumably with error correction/detection so that you can assume that you either see what the other end sent you, in the order in which it sent it (but not preserving buffer/packet/record boundaries!), or you get a "broken connection" error. Now, we may be in violent agreement here -- the transports I am thinking of can certainly use any of the mechanisms you list as underlying abstraction. But I wouldn't call it a transport unless it had standardized semantics and a standardized interface with the protocol. (For datagrams, we need slightly different abstractions, with different guarantees and semantics. But, again, all datagram transports should be more or less interchangeable.)
Yeah, well, but in practice I expect that layering transports on top of each other is rare, and using platform specific transport implementations is by far the common case. (Note that in theory you could layer SSL over any unencrypted transport; but in practice (a) few people need that, and (b) the ssl module doesn't support this -- hence I am comfortable with treating SSL as another platform-specific transport.)
I don't know enough about JSON-RPC (shame on me!) but this sounds very reasonable.
True. There's one important issue here: *constructing* the stack is not up to the event loop. It is totally fine if the HTTP-based transport is a 3rd party package that exports a function to set up the stack, given an event loop and a protocol to run on top (JSON-RPC in this example). This function can have a custom signature that is not compatible with any other transport-creating APIs in existence. (In fact this is why I renamed create_transport() to create_connection() -- the standardized API just has methods for creating internet connections.)
I'm not even sure what you mean by the protocol API. From the PEP's POV, the "protcol API" is just the methods that the transport calls (connection_made(), data_received(), etc.) and those certainly *are* supposed to be standardized.
So, for example, Greg's request for the ability to pass callbacks rather than needing particular method names
Hm, I have yet to respond to Greg's message, but I'm not sure that's a reasonable request.
Well, except that you can't just pass CallbackProtocol where a protocol factory is required by the PEP -- you'll have to pass a lambda or partial function without arguments that calls CallbackProtocol with some arguments taken from elsewhere. No big deal though.
That sounds like an intriguing idea which I'd like to explore in the distant future. One point of light: a transport probably already is acceptable as a binary *output* stream, because its write() method is not a coroutine. (This is intentional.) But doing the same for input is harder.
Right. (TBH, I'm often not sure whether you are just explaining the PEP's philosophy or trying to propose changes... Sorry for the confusion this may cause.) -- --Guido van Rossum (python.org/~guido)
Guido van Rossum wrote:
Something smells wrong to me about APIs that require protocol factories. I don't see what advantage there is in writing create_connection(HTTPProtocol, "some.where.net", 80) as opposed to just writing something like HTTPProtocol(TCPTransport("some.where.net", 80)) You're going to have to use the latter style anyway to set up anything other than the very simplist configurations, e.g. your earlier 4-layer protocol stack example. So create_connection() can't be anything more than a convenience function, and unless I'm missing something, it hardly seems to add enough convenience to be worth the bother. -- Greg
On Fri, Jan 18, 2013 at 3:59 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Glyph should really answer this one. Personally I don't feel strongly either way for this case. There may be an advantage to not calling the protocol factory if the connection can't be made (in which case the Future returned by create_connection() has the exception). -- --Guido van Rossum (python.org/~guido)
On Fri, 18 Jan 2013 16:12:29 -0800 Guido van Rossum <guido@python.org> wrote:
Except that you probably want the protocol to outlive the transport if you want to deal with reconnections or connection failures, and therefore: TCPClient(HTTPProtocol(), ("some.where.net", 80)) Regards Antoine.
Antoine Pitrou wrote:
I don't see how to generalise that to more complicated protocol stacks, though. For dealing with re-connections, it seems like both the protocol *and* the transport need to outlive the connection failure, and the transport needs a reconnect() method that is called by a protocol that can deal with that situation. Reconnection can then propagate along the whole chain. -- Greg
Just like there's no reason for having a protocol without a transport, it seems like there's no reason for a transport without a connection, and that separating the two might further normalize differences between client and server channels Shane Green www.umbrellacode.com 805-452-9666 | shane@umbrellacode.com On Jan 18, 2013, at 8:16 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
On Jan 18, 2013, at 4:12 PM, Guido van Rossum <guido@python.org> wrote:
Glyph should really answer this one.
Thanks for pointing it out to me, keeping up with python-ideas is always a challenge :).
On Fri, Jan 18, 2013 at 3:59 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
For starters, nothing "smells wrong" to me about protocol factories. Responding to this kind of criticism is difficult, because it's not substantive - what's the actual problem? I think that some Python programmers have an aversion to factories because a common path to Python is flight from Java environments that over- or mis-use the factory pattern.
Guido mentioned one advantage already; you don't have to create the protocol object if the connection fails, so your protocol objects are real honest-to-goodness connections, not "well, maybe there's a connection or maybe there'll be a connection later". To be fair, this is rarely of practical utility, but in edge cases where you are doing something like, "simultaneously try to connect to these 1000 hosts, and give up on all outstanding connections when the first 3 connections succeed", being able to avoid all the construction overhead for your protocols if they're not going to be used is nice. There's a more pressing issue of correctness though: even if you create the protocol in advance, you really don't want to tell it about the transport until the transport truly exists. The connection to some.where.net (by which I mean, ahem, "somewhere.example.com"; "where.net" will not thank you if you ignore BCP 32 in the documentation or examples) might fail, and if the client wants to issue a client greeting, it should not have access to its half-formed transport before that failure. Of course, it's possible to present an API that works around this by buffering writes issued before the connection is established, and by the protocol waiting for the connection_made callback before actually doing its work. Finally, using a factory also makes client-creating and server-creating code more symmetrical, since you clearly need a protocol factory in the listening-socket case. If your main example protocol is HTTP, this doesn't make sense*, but once you start trying to do things like SIP or XMPP, where the participants in a connection are really peers, having the structure be similar is handy. In the implementation, it's nice to have things set up this way so that the order of the protocol<->transport symmetric setup is less important and by the time the appropriate methods are being invoked, everybody knows about everybody else. The transport can't really have a reference to the protocol in the protocol's constructor. *: Unless you're doing this, of course <http://wiki.secondlife.com/wiki/Reverse_HTTP>. However, aside from the factory-or-not issue, the fact that TCPTransport's name implies that it is both (1) a class and (2) the actual transport implementation, is more problematic. TCPTransport will need multiple backends for different multiplexing and I/O mechanisms. This is why I keep bringing up IOCP; this is a major API where the transport implementation is actually quite different. In Twisted, they're entirely different classes. They could probably share a bit more implementation than they do and reduce a little duplication, but it's nice that they don't have to. You don't want to burden application code with picking the right one, and it's ugly to smash the socket-implementation-selection into a class. (create_connection really ought to be a method on an event-loop object of some kind, which produces the appropriate implementation. I think right now it implicitly looks it up in thread-local storage for the "current" main loop, and I'd rather it were more explicit, but the idea is the same.) Your example is misleadingly named; surely you mean TCPClient, because a TCPTransport would implicitly support both clients and servers - and a server would start with a socket returned from accept(), not a host and port. (Certainly not a DNS host name.) create_connection will actually need to create multiple sockets internally. See <http://tools.ietf.org/html/rfc3493> covers this, in part (for a more condensed discussion, see <https://twistedmatrix.com/trac/ticket/4859>).
I don't see how this is true. I've written layered protocols over and over again in Twisted and never wanted to manually construct the bottom transport for that reason.* In fact, the more elaborate multi-layered structures you have to construct when a protocol finishes connecting, the more you want to avoid being required to do it in advance of actually needing the protocols to exist. *: I _have_ had to manually construct transports to deal with some fiddly performance-tuning issues, but those are just deficiencies in the existing transport implementation that ought to be remedied.
*Just* implementing the multiple-parallel-connection-attempts algorithm required to deal with the IPv6 transition period would be enough convenience to be worth having a function, even if none of the other stuff I just wrote applied :). -glyph
Glyph wrote:
I'm not averse to using the factory pattern when it genuinely helps. I'm questioning whether it helps enough in this case to be worth using.
I would suggest that merely instantiating a protocol object should be cheap enough that you don't normally care. Any substantive setup work should be done in the connection_made() method, not in __init__(). Transports are already a "maybe there's a connection" kind of deal, otherwise why does connection_made() exist at all?
Which it seems to me is the way *all* protocols should be written. If necessary, you could "encourage" people to write them this way by having a transport refuse to accept any writes until the connection_made() call has occurred.
They don't have to be classes, they could be functions: create_http_protocol(create_tcp_transport("hammerme.seeificare.com", 80)) The important thing is that each function concerns itself with just one step of the chain, and chains of any length can be constructed by composing them in the obvious way.
Maybe. Or maybe the constructor could be called in more than one way -- create_tcp_transport(host, port) on the client side and create_tcp_transport(socket) on the server side.
Couldn't all that be handled inside the transport?
So what does the code for setting up a multi-layer stack look like? How does it make use of create_connection()? Also, what does an implementation of create_connection() look like that avoids creating the protocol until the connection is made? It seems tricky, because the way you know the connection is made is that it calls connection_made() on the protocol. But there's no protocol yet. So you would have to install a temporary protocol whose connection_made() creates the real protocol. That sounds like it could be even more overhead than just creating the real protocol in the first place, as long as the protocol doesn't do any work until its connection_made() is called. -- Greg
On Fri, Jan 18, 2013 at 8:23 PM, Glyph <glyph@twistedmatrix.com> wrote:
I think the smell is that the factory is A) only used once and B) invoked without adding any additional arguments that weren't available when the factory was passed in, so there's no clear reason to defer creation of the protocol. I think it would make more sense if the transport were passed as an argument to the factory (and then we could get rid of connection_made as a required method on Protocol, although libraries or applications that want to separate protocol creation from connection_made could still do so in their own factories). -Ben
On Jan 19, 2013, at 9:32 AM, Ben Darnell <ben@bendarnell.com> wrote:
The problem with creating the protocol with the transport as an argument to its constructor is that in order to behave correctly, the transport needs to know about the protocol as well; so it also wants to be constructed with a reference to the protocol to *its* constructor. So adding a no-protocol-yet case adds more edge-cases to every transport's implementation. All these solutions are roughly isomorphic to each other, so I don't care deeply about it. However, my proposed architecture has been in use for a decade in Twisted without any major problems I can see. I'm not saying that Twisted programs are perfect, but it would *really* be useful to discuss this in terms of problems you can identify with the humungous existing corpus of Twisted-using code, and say "here's a problem that develops in some programs due to the sub-optimal shape of this API". Unnecessary class definitions, for example, or a particular type of bug; something like that. For example, I can identify several difficulties with Twisted's current flow-control setup code and would not recommend that it be copied exactly. Talking about how the code smells or what might hypothetically make more sense is just bikeshedding. -glyph
On Sun, Jan 20, 2013 at 8:53 AM, Glyph <glyph@twistedmatrix.com> wrote:
But the trade-off in separating protocol creation from notification of the connection is that it means every *protocol* has to be written to handle the "no connection yet" gap between __init__ and the call to connection_made. However, if we instead delay the call to the protocol factory until *after the connection is made*, then most protocols can be written assuming they always have a connection (at least until connection_lost is called). A persistent protocol that spanned multiple connect/reconnect cycles could be written such that you passed "my_protocol.connection_made" as the protocol factory, while normal protocols (that last only the length of a single connection) would pass "MyProtocol" directly. At the transport layer, the two states "has a protocol" and "has a connection" could then be collapsed into one - if there is a connection, then there will be a protocol, and vice-versa. This differs from the current status in PEP 3156, where it's possible for a transport to have a protocol without a connection if it calls the protocol factory well before calling connection_made. Now, it may be that *there's a good reason* why conflating "has a protocol" and "has a connection" at the transport layer is a bad idea, and thus we actually *need* the "protocol creation" and "protocol association with a connection" events to be distinct. However, the PEP currently doesn't explain *why* it's necessary to separate the two, hence the confusion for at least Greg, Ben and myself. Given that new protocol implementations should be significantly more common than new transport implementations, there's a strong case to be made for pushing any required complexity into the transports. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Sat, Jan 19, 2013 at 5:51 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
That doesn't strike me as a problematic design. I've seen it plenty of times.
Well, almost. connection_made() would have to return self to make this work. But we could certainly use add some other method that did that. (At first I thought it would be harder to pass other parameters to the constructor for the non-reconnecting case, but the solution is about the same as before -- use a partial function or a lambda that takes a protocol and calls the constructor with that and whatever other parameters it wants to pass.)
This doesn't strike me as important. The code I've written for Tulip puts most of the connection-making code outside the transport, and the transport constructor is completely private. Every transport implementation is completely free in how it works, and every event loop implementation is free to put as much or as little of the connection set-up in the transport as it wants to. The same is true for transports written by users (and there will be some of these). The *only* things we care about for transports is that the thing passed to the protocol's connection_made() has the methods specified by the PEP (write(), writelines(), pause(), resume(), and a few more). Also, it does not matter one iota whether it is the transport or some other entity that calls the protocol's methods (connection_made(), data_received(), etc.) -- the only thing that matters is the order in which they are called. IOW, even though a transport may "have" a protocol without a connection, nobody should care about that state, and nobody should be calling its methods (again, write() etc.) in that state. In fact, nobody except event loop internal code should ever have a reference to a transport in that state. (The transport that is returned by create_connection() is fully connected to the socket (or whatever might takes its place) as well as to the protocol.) I think we can make the same assumptions for transports implemented by user code.
So, your whole point here seems to be that you'd rather see the PEP specify that the sequence when a connection is made is protocol = protocol_factory(transport) rather than protocol = protocol_factory() protocol.connection_made(transport) I looked in the Tulip code to see whether this would cause any problems. I think it could be done, but the solution would feel a little awkward to me, because currently the protocol's connection_made() method is not called directly by the transport: it is called indirectly via the event loop's call_soon() method. So using your approach the transport wouldn't have a protocol attribute until this callback is called -- or we'd have to change things to call it directly rather than via call_soon(). Now I'm pretty sure I can prove that nothing will be referencing the protocol *before* the connection_made() call is actually made, and also that directly calling it instead of using call_soon() is fine. But nevertheless the transport code would feel a little harder to reason about.
TBH I don't see the protocol implementation getting any simpler because of this. There is some protocol initialization code that doesn't depend on the transport, and some that does. Using your approach, these all go in __init__(). Using the PEP's current proposal, the latter go in a separate method, connection_made(). But using your approach, writing the lambda or partial function that calls the constructor with the right arguments (to be passed as protocol_factory) becomes a tad more complex, since now it must take a transport argument. On the third hand, rigging things so that a pre-existing protocol instance can be reused becomes a little harder to figure out, since you have to write a helper method that takes a transport and returns the protocol (i.e., self). All in all I see it as six of one, half a dozen of the other, and I am happy with Glyph's testimony that the Twisted design works well in practice. -- --Guido van Rossum (python.org/~guido)
On Sun, Jan 20, 2013 at 2:35 PM, Guido van Rossum <guido@python.org> wrote:
When the two are separated without a clear definition of what else can happen in between, *every other method on the protocol* needs to cope with the fact that other calls to protocol methods may happen in between the call to __init__ and the call to connection_made - you simply can't write a protocol without dealing with that problem. As you correctly figured out, my specific proposal was to move from: protocol = protocol_factory() protocol.connection_made(transport) To a single event: protocol = protocol_factory(transport) The *reason* I wanted to do this is that I *don't understand* what may happen to my protocol implementation between construction and the call to make_connection. Your description of the current implementation actually worries me, as it suggests to me that when I get a (transport, protocol) pair back from a call to "create_connection", "connection_made" may *not* have been called yet - the protocol may be in exactly the state I am worried about, because the event loop is sending the notification in a fire-and-forget fashion, instead of waiting until the call is complete: protocol = protocol_factory() loop.call_soon(protocol.connection_made, transport) # The protocol isn't actually fully initialized here... However, that description also made me realise why two distinct operations are needed, so I'd like to change my suggestion to the following: protocol = factory() yield from protocol.connection_made(transport) # Or callback equivalent The protocol factory would still be used to create the protocol object. However, the PEP would be updated to make it clear that immediately after creation the *only* permitted method invocation on the result is "connection_made", which will complete the protocol initialization process. The connection_made event handler would be redefined to return a *Future* (or equivalent object) rather than completing synchronously. create_connection would then call connection_made and *wait for it to finish*, rather than using call_soon in a fire-and-forget fashion. The advantage of this is that the rationale for the various possible states become clear: - the protocol factory is invoked synchronously, and is thus not allowed to perform any blocking actions (but may trigger "fire-and-forget" operations) - connection_made is invoked asynchronously, and is thus able to wait for various operations - a protocol returned from create_connection is certain to have had connection_made already called, thus a protocol implementation may safely assume in other methods that both __init__ and connection_made will have been called during the initialization process. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Jan 19, 2013, at 10:13 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Nope. You only have to deal with the methods that the transport will call on the protocol in that state, since nothing else has a reference to it yet. Except the transport won't call them in that state, so... still nope. Again: there's enormous corpus of Twisted code out there that is written this way, you can go look at that code to see how it deals with the problem you've imagined, which is to say: it doesn't. It doesn't need to. Now, if you make the change you're proposing, and tie together the protocol's construction with the transport's construction, so that you end up with protocol(transport(...)), this means that the protocol will immediately begin interacting with the transport in this vague, undefined, not quite connected state, because, since the protocol didn't even exist at the time of the transport's construction, the transport can't possibly have a reference to a protocol yet. And the side of the protocol that issues a greeting will necessarily need to do transport.write(), which may want to trigger a notification to the protocol of some kind (flow control?), and where will that notification go? It needs to be solved less often, but it's a much trickier problem to solve. There are also some potential edge-cases where the existing Twisted-style design might be nicer, like delivering explicit TLS handshake notifications to protocols which support them in the vague state between protocol construction and connection_made, but seeing as how I still haven't gotten around to implementing that properly in Twisted, I imagine it will be another 10 years before Tulip is practically concerned with it :). Finally, I should say that Guido's point about the transport constructor being private is actually somewhat important. We've been saying 'transport(...)' thus far, but in fact it's more like 'SocketTransport(loop, socket)'. Or perhaps in the case of a pipe, 'PipeTransport(loop, readfd, writefd)'. In the case of an actual outbound TCP connection with name resolution, it's 'yield from make_outgoing_tcp_transport(loop, hostname, port)'. Making these all methods that hide the details and timing of the transport's construction is a definite plus. -glyph
On Sun, Jan 20, 2013 at 4:51 PM, Glyph <glyph@twistedmatrix.com> wrote:
Yes, after Guido explained how tulip was currently handling this, I realised that the problem was mostly one of documentation. However, I think there is one key bug in the current implementation, which is that create_connection is returning *before* the call to "connection_made" is completed, thus exposing the protocol in an incompletely initialised state.
Yes, I didn't have a problem with that part - it was just the lack of clear explanation of the different roles of the protocol constructor and the connection_made callback that I found problematic. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Jan 19, 2013, at 11:18 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Aah. Yes, I think you're right about that being a bug. There are probably some docs in Twisted that could be improved to explain that this ordering is part of our analogous interface's contract...
I wasn't clear if you were arguing against it; I just wanted to make it clear :). -glyph
Glyph wrote:
You still haven't explained why the protocol can't simply refrain from doing anything with the transport until its connection_made() is called. If a transport is always to be assumed ready-to-go as soon as it's exposed to the outside world, what is the point of having connection_made() at all? -- Greg
On Sat, 19 Jan 2013 20:35:04 -0800 Guido van Rossum <guido@python.org> wrote:
This is just not true. When the connection breaks, the protocol still has a reference to the transport and may still be trying to do things with the transport (because connection_lost() has not been called yet). Regards Antoine.
On Sun, Jan 20, 2013 at 4:36 AM, Antoine Pitrou <solipsis@pitrou.net> wrote:
That's a different case though. There once *was* a connection. You are right that the transport needs to protect itself against the protocol making further calls to the transport API in this case. Anyway, I think Nick is okay with the separation between the protocol_factory() call and the connection_made() call, as long as the future returned by create_connection() isn't marked done until the connection_made() call returns. That's an easy fix in the current Tulip code. It's a little harder though to fix up the PEP to clarify all this... -- --Guido van Rossum (python.org/~guido)
On Mon, Jan 21, 2013 at 5:03 AM, Guido van Rossum <guido@python.org> wrote:
Right, I understand what the separate method enables now. I think one way to make it clearer in the PEP is to require that "connection_made" return a Future or coroutine, rather than being an ordinary method returning None. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Sun, Jan 20, 2013 at 11:52 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Hm. This would seem to introduce Futures / coroutines at the wrong level (I want to allow protocol implementers to use them, but not require them). If connection_made() wants to initiate some blocking I/O, it is free to do so, but it ought to wrap that in a Task. If the class needs completion of this task to be a prerequisite for handling data passed to a subsequent data_received() call, it will need to devise some buffering and/or locking scheme that's outside the scope of the PEP. Note that I am also hoping to produce a more coroutine-oriented style for writing protocols. The main piece of code for this already exists, the StreamReader class (http://code.google.com/p/tulip/source/browse/tulip/http_client.py?r=b1028ab0...), but I need to think about how to hook it all together nicely (for writing, the transport's API is ready to be used by coroutines). -- --Guido van Rossum (python.org/~guido)
On Fri, Jan 18, 2013 at 12:08 AM, Paul Moore <p.f.moore@gmail.com> wrote:
No, it doesn't need to be a method on the event loop at all. It can just be a function in a different package; it can use events.get_current_event_loop() to reference the event loop. -- --Guido van Rossum (python.org/~guido)
On 18 January 2013 21:24, Guido van Rossum <guido@python.org> wrote:
Aargh. I'm confused again! (I did warn you about dumb questions, didn't I? :-)) The event loop implementation contains the code that does the OS-level poll for events to process. (In tulip, that is handled by the selector object, but that's not mentioned in the PEP so I assume it should be considered an implementation detail). So, the event loop has to define what types of (OS-level) objects can be registered. At the moment, event loops only handle sockets (via select/poll/etc) and even the raw add_reader methods are not for end user use. So a standalone create_corba_connection function can certainly get the event loop using get_current_event_loop(), but it has no means of asking the event loop to poll the CORBA connection it creates for new messages. Without direct access to the selector (or equivalent) it can't add the extra event source. (Unless that source is a pollable file descriptor and it's willing to play with the optional add_reader methods, but that's not a "new event source" then...) The same problem will likely occur if you try to integrate Windows GUI events (you check for a GUI message by calling a Windows API). I don't think this matters except in obscure cases (it's likely a huge case of YAGNI) but I genuinely don't understand how you can say that create_corba_connection() could be written as a standalone function, and yet that create_connection() has to be a method of the event loop. That's what I'm getting at when I keep saying that I see you treating sockets as "special". There's clearly something I'm missing in your thinking, and it keeps tripping me up. Paul.
On Fri, Jan 18, 2013 at 2:32 PM, Paul Moore <p.f.moore@gmail.com> wrote:
Well, *on UNIX* the event loop also handles other file descriptors, and there's nothing to actually *prevent* an end user using add_reader. It just may not work when their code is run on Windows, but then it probably won't run on Windows anyway. :-)
Right, unless it is in on the conspiracy between the event loop and the selector (IOW if it is effectively aware and/or part of the event loop implementation for the specific platform).
Let's say that you are thinking through the example much farther than I had intended... :-)
Let's assume that create_corba_connection() actually *can* be written using add_reader(), but only on UNIX. So the app is limited to UNIX, and in that context create_corba_connection() can be a function in another package. It's not so much that create_connection() *must* be a method on the event loop. It's just that I *want* it to be a method on the event loop so you will be able to write user code that is portable between UNIX and Windows. It will call create_connection(), which is a portable API with two platform-specific implementations; on Windows (when using IOCP) it will return an instance of, say, _IocpSocketTransport(), while on UNIX it returns a _UnixSocketTransport() instance. But we have no hope of making create_corba_connection() on Windows (in my example -- please just play along) and hence there is no need to make it a method of the event loop. -- --Guido van Rossum (python.org/~guido)
On Wed, Jan 16, 2013 at 7:12 AM, Paul Moore <p.f.moore@gmail.com> wrote:
This is a great use case. The right approach would probably be to define a new Transport (and an event loop method to create one) that wraps pipes going into and out of a subprocess. The new method would have a standard API (probably similar to that of subprocess), whereas there would be different implementations of the Transport based on platform and event loop implementation (similar to the way the subprocess module has quite different implementations). Can you check out the Tulip source code (code.google.com/p/tulip) and come up with a patch to do this? I'll gladly review it. It's fine to only cover the UNIX case for now. -- --Guido van Rossum (python.org/~guido)
On 16 January 2013 17:52, Guido van Rossum <guido@python.org> wrote:
I'll have a look. There *is* one problem, though - I imagine it will be relatively easy to put something together that works on Unix, as waiting on pipes is covered by the existing select/poll mechanisms. But I'm on Windows, so I won't be able to test it. And on Windows, there's no mechanism in place to wait on arbitrary filehandles, so the process wait mechanism is a much harder nut to crack. Chicken and egg problem... Maybe I'll start by looking at waiting on arbitrary filehandles, and use that to build the process approach. Unfortunately, I don't think IOCP is any more able to wait on arbitrary files than select - see my followup to an older thread on Richard's work there. Or maybe I'll set up a hacking environment in a Linux VM or something. That'd be a fun experience in any case. I'll have to get my brain round the existing spec as well. I'm finding it hard to understand why there are so many methods on the event loop that are specific to particular use cases (for this example, your suggested method to create the new type of Transport). My instinct says that this should *also* be a good test case for a user coming up with a new type of "event source" and wanting to plug it into the event loop. Having to add a new method to the event loop seems to imply this isn't possible. OK, off to do a lot of spec reading and then some coding. With luck, you'll be patient with dumb questions from me on the way :-) Thanks, Paul
On Wed, Jan 16, 2013 at 8:10 PM, Paul Moore <p.f.moore@gmail.com> wrote:
Dealing with subprocesses on Windows in a non-blocking way is a royal pain. As far as I know, the only option is to use named pipes and block on them using a thread pool. A few years back I wrote something that did this, see the link below. However it ain't pretty.. https://bitbucket.org/geertj/winpexpect/src/tip/lib/winpexpect.py Regards, Geert
On Wed, Jan 16, 2013 at 10:10 AM, Paul Moore <p.f.moore@gmail.com> wrote:
What does the subprocess module do on Windows? (I'm in the reverse position, although I have asked the kind IT folks at Dropbox to provide me with a Windows machine.)
I'm eagerly awaiting Richard's response. AFAIK handles on Windows *are* more general than sockets...
This is mainly so that the event loop implementation can control the Transport class. Note that it isn't enough to define different Transport classes per platform -- on a single platform there may be multiple event loop implementations (e.g. on Windows you can use Select or IOCP) and these may need different Transport implementations. SO this must really be under control of the event loop object.
If the user is okay with solving the problem only for their particular platform and event loop implementation they don't need to add anything to the event loop. But for transports that make it into the PEP, it is essential that alternate implementations (e.g. one that proxies a Twisted Reactor) be in control of the Transport construction.
OK, off to do a lot of spec reading and then some coding. With luck, you'll be patient with dumb questions from me on the way :-)
I will be! -- --Guido van Rossum (python.org/~guido)
On 16/01/2013 6:21pm, Guido van Rossum wrote:
I'm eagerly awaiting Richard's response. AFAIK handles on Windows *are* more general than sockets...
I would like to modify subprocess on Windows to use file-like objects which wrap overlapped pipe handles. Then doing async work with subprocess would become relatively straight forward, and does not really require tulip or IOCP. -- Richard
On Wed, Jan 16, 2013 at 12:45 PM, Greg Ewing <greg.ewing@canterbury.ac.nz>wrote:
Tulip on UNIX already wraps pipes (and ptys, and certain other things), since the add_reader() API takes any filedescriptor (though it makes no sense for disk files because those are always considered readable). The issue is that on other platforms (read: Windows) you have to do something completely different, and hook it up to the native (IOCP) async eventloop differently. The Transport/Protocol abstraction however would be completely appropriate in both cases though (or a slightly modified version that handles stdout/stderr separately). So, just like the subprocess module contains two completely disjoint implementations for UNIX and Windows, implementing the same API, PEP 3156 could also have a standard API for running a subprocess connected with async streams connected to stdin, stdout, stderr, backed by different implementations. -- --Guido van Rossum (python.org/~guido)
On 16 January 2013 18:21, Guido van Rossum <guido@python.org> wrote:
OK, I'm reading the PEP through now. I'm happy with the basics of the event loop, and it seems fine to me. When I reached create_transport, I had to skip ahead to the definitions of transport and protocol, as create_transport makes no sense if you don't know about those. Once I've read that, though, the whole transport/protocol mechanism seems to make reasonable sense to me. Although the host and port arguments to create_transport are clearly irrelevant to the case of a transport managing a process as a data source. So (a) I see why you say I'd need a new transport creation method, but (b) it strikes me that something more general that covered both cases (and any others that may come up later) would be better. On the other hand, given the existence of create_transport, I'm now struggling to understand why a user would ever use add_reader/add_writer rather than using a transport/protocol. And if they do have a reason to do so, why does a similar reason not apply to having an add_pipe type of method for waiting on (subprocess) pipes? In general, it still feels to me like the socket use case is being treated as "special", and other data sources and sinks (subprocesses being my use case, but I'm sure others exist) are either second-class or require a whole set of their own specialised methods, which isn't practical. As a strawman type of argument in favour of extensibility, consider a very specialist user with a hardware device that sends input via (say) a serial port. I can easily imagine that user wanting to plug his device data into the Python event loop. As this is a very specialised area, I wouldn't expect the core code to be able to help, but I would expect him to be able to write code that plugs into the standard event loop seamlessly. Ideally, I'd like to use the subprocess case as a proof that this is practical. Does that make sense? Paul.
On 17 January 2013 12:23, Paul Moore <p.f.moore@gmail.com> wrote:
Thinking about this some more. The key point is that for any event loop there can only be one "source of events" in terms of the thing that the event loop checks when there are no pending tasks. So the event loop is roughly: while True: process_ready_queue() new_events = block_on_event_source(src, timeout=N) add_to_ready_queue(new_events) add_timed_events_to_ready_queue() The source has to be a unique object, as there's an OS-level wait in there, and you can't do two of them at once. As things stand, methods like add_reader on the event loop object should really be methods on the event source object (and indeed, that's more or less what Tulip does internally). Would it not make more sense to explicitly expose the event source? This is (I guess) what the section "Choosing an Event Loop Implementation" in the PEP is about. But if the event source is a user-visible object, methods like add_reader would no longer be optional event loop methods, but rather they would be methods of the event source (but only for those event sources for which they make sense). The point here is that there's a lot of event loop machinery (ready queue, timed events, run methods) that are independent of the precise means by which you poll the OS to ask "has anything interesting happened?" Abstracting out that machinery would seem to me to make the design cleaner and more understandable. Other benefits - our hypothetical person with a serial port device can build his own event source and plug it into the event loop directly. Or someone could offer a multiplexer that combines two separate sources by running them in different threads and merging the output on a queue (that may be YAGNI, though). This is really just something to think about while I'm trying to build a Linux development environment so that I can do a Unix proof of concept. Once I get started on that, I'll think about the protocol/transport stuff. Paul
(I'm responding to two separate messages in one response.) On Thu, Jan 17, 2013 at 4:23 AM, Paul Moore <p.f.moore@gmail.com> wrote:
Whoops, I should fix the order in the PEP, or at least insert forward references.
This is why there is a TBD item suggesting to rename create_transport() to create_connection() -- this method is for creating the most common type of transport only, i.e. one that connects a client to a server given by host and port.
add_reader and friends exist for the benefit of Transport implementations. The PEP even says that not all event loops need to implement these (though on UNIXy systems it is better if they do, and I am considering removing or weakening this language. Because on UNIX pipes are just file descriptors, and work fine with select()/poll()/etc., there is no need for add_pipe() (assuming that API would take an existing pipe filedescriptor and a callback), since add_reader() will do the right thing. (Or add_writer() for the other end.)
Well, sockets are treated special because on Windows they *are* special. At least the select() system call only works for sockets. IOCP supports other types of unusual handles, but the ways to create handles you can use with it are mostly custom. Basically, if you want to write code that works both on Windows and on UNIX, you have to limit yourself to sockets. (And you shouldn't use add_reader and friends either, because that limits you to the SelectSelector, whereas if you use the transport/protocol API you will be compatible with either that or IOCPSelector.)
Yes, it does make sense, but you have to choose whether to do it on Windows or on UNIX. If you use UNIX, presumably your serial port is accessible via a file descriptor that works with select/poll/etc. -- if it doesn't, you are going to have a really hard time integrating it with the event loop, you may have to use a separate thread that talks to the device and sends the data to the event loop over a pipe or something. On Windows, I have no idea how it would work, but I presume that serial port drivers are somehow hooked up to "handles" and "waitable events" (or whatever the Microsoft terminology is -- I am about to get educated about this) and then presumably it will integrate nicely with IOCP (but not with Select). I think that for UNIX, hooking a subprocess up to a transport should be easy enough (except perhaps for the stdout/stderr distinction), and your transport should use add_reader/writer. For Windows I am not sure but you can probably crib the details from the Windows-specific code in subprocess.py in the stdlib. On Thu, Jan 17, 2013 at 6:35 AM, Paul Moore <p.f.moore@gmail.com> wrote:
Right, that's the idea.
The problem with this idea is (you may have guessed it by now :-) ... Windows. On Windows, at least when using a (at this moment purely hypothetical) IOCP-based implementation of the event loop, there will *not* be an underlying Selector object. Please track down discussion of IOCP in older posts on this list. IOCP requires you to use a different paradigm, which is supported by the separate methods sock_recv(), sock_sendall() and so on. For I/O objects that are not sockets, different methods are needed, but the idea is the same: you specify the I/O, and you get a callback when it is done. This in contrast with the UNIX selector, where you specify the file descriptor and I/O direction, and you get a callback when you can read/write without blocking. This is why the event loop has the higher-level transport/protocol-based APIs: an IOCP implementation of these creates instances of a completely different transport implementation, which however have the same interface and *meaning* as the UNIX transports (e.g. the transport created by create_connection() connects to a host and port over TCP/IP and calls the protocol's connection_made(), data_received(), connection_lost() methods). So if you want a transport that encapsulates a subprocess (instead of a TCP/IP connection), and you want to support both UNIX and Windows, you have to provide (at least) two separate implementations: one on UNIX that uses add_reader() and friends, and one on Windows that uses (I don't know what, but something). Each of these implementations by itself is dependent on the platform (and the specific event loop implementation); but together they cover all supported platforms. If you develop this as 3rd party code, and you want your users not to have to write platform-specific code, you have to write a "start subprocess" function that inspects the platform (and the event loop implementation) and then imports and instantiates the right transport implementation for the platform. If we want to add this to the PEP, the right thing is to add a "start subprocess" method to the event loop API (which can be identical to the start subprocess function in your 3rd party package :-).
It is abstracted out in the implementation, but I hope I have explained with sufficient clarity why it should not be abstracted out in the PEP: the Selector abstraction only works on UNIX (or with sockets on Windows). Also note a subtlety in the PEP: while it describes a platform-independent API, it doesn't preclude that some parts of that API may have platform-specific behaviors -- for example, add_reader() may only take sockets on Windows (and in Jython, I suspect, where select() only works with sockets), but takes other file descriptors on UNIX, so you can implement your own subprocess transport for UNIX. Similarly, the PEP describes the interface between transports and protocols, but does not give you a way to construct a transport except for TCP/IP connections. But the abstraction is usable for other purposes too, and this is intentional! (E.g. you may be able to create a transport that uses a subprocess running ssh to talk to a remote server, which might be used to "tunnel" HTTP, so it would make sense to connect this custom transport with a standard HTTP protocol implementation.)
Other benefits - our hypothetical person with a serial port device can build his own event source and plug it into the event loop directly.
I think I've answered that above.
I think there are Twisted reactor implementations that do things like this. My hope is that a proxy between the Twisted reactor and the PEP 3156 interface will enable this too -- and the event loop APIs for working with transports and protocols are essential for this purpose. (Twisted has a working IOCP reactor, FWIW.)
I think it would be tremendously helpful if you tried to implement the UNIX version of the subprocess transport. (Note that AFAIK Twisted has one of these too, maybe you can get some implementation ideas from them.) -- --Guido van Rossum (python.org/~guido)
Hmm, there may still be something to the idea of clearly separating out "for everyone" and "for transports" methods. Even if that's just a split in the documentation, similar to the "for everyone" vs "for the executor" split in the concurrent.futures implementation. -- Sent from my phone, thus the relative brevity :)
On 17 January 2013 19:10, Guido van Rossum <guido@python.org> wrote:
You were right. In starting to do so, I found out that my thinking has been solely based on a callback style of programming (users implement protocol classes and code the relevant "data received" methods themselves). From looking at some of the sample code, I see that this is not really the intended usage style. At this point my head exploded. Coroutines, what fun! I am now reading the sample code, the section of the PEP on coroutines, and the mailing list threads on the matter. I may be some time :-) (The technicalities of the implementation aren't hard - it's just a data_received type of protocol wrapper round a couple of pipes. It's the usability and design issues that matter, and they are strongly affected by "intended usage"). Paul PS From the PEP, it seems that a protocol must implement the 4 methods connection_made, data_received, eof_received and connection_lost. For a process, which has 2 output streams involved, a single data_received method isn't enough. I see two options - having 2 separate protocol classes involved, or having a process protocol with a different interface. Neither option seems obviously best, although Twisted appears to use different protocol types for different types of transport. How critical is the principle that there is a single type of protocol to the PEP?
On Thu, Jan 17, 2013 at 3:44 PM, Paul Moore <p.f.moore@gmail.com> wrote:
Right, this is a very good observation.
Not critical at all. The plan for UDP (datagrams in general) is to have different protocol methods as well. TBH I would be happy with a first cut that only deals with stdout, like os.popen(). :-) Note that I am intrigued by this problem as well and may be hacking up a version for myself in my spare time. -- --Guido van Rossum (python.org/~guido)
Paul Moore wrote:
It looks like there would have to be at least two Transport instances involved, one for stdin/stdout and one for stderr. Connecting them both to a single Protocol object doesn't seem to be possible with the framework as defined. You would have to use a couple of adapter objects to translate the data_received calls into calls on different methods of another object. This sort of thing would be easier if, instead of the Transport calling a predefined method of the Protocol, the Protocol installed a callback into the Transport. Then a Protocol designed for dealing with subprocesses could hook different methods of itself into a pair of Transports. Stepping back a bit, I must say that from the coroutine viewpoint, the Protocol/Transport stuff just seems to get in the way. If I were writing coroutine-based code to deal with a subprocess, I would want to be able to write coroutines like def handle_output(stdout): while 1: line = yield from stdout.readline() if not line: break mungulate_line(line) def handle_errors(stderr): while 1: line = yield from stderr.readline() if not line: break complain_to_user(line) In other words, I don't want Transports or Protocols or any of that cruft, I just want a simple pair of async stream objects that I can read and write using yield-from calls. There doesn't seem to be anything like that specified in PEP 3156. It does mention something about implementing a streaming buffer on top of a Transport, but in a way that makes it sound like a suggested recipe rather than something to be provided by the library. Also it seems like a lot of layers of overhead to go through. On the whole, in PEP 3156 the idea of providing callback-based interfaces with yield-from-based ones built on top has been pushed way further up the stack than I imagined it would. I don't want to be *forced* to write my coroutine code at the level of Protocols; I want to be able to work at a lower level than that. -- Greg
On Thu, Jan 17, 2013 at 11:17 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
So far this makes sense. But for this specific case there's a simpler solution -- require the protocol to support a few extra methods, in particular, err_data_received() and err_eof_received(), which are to stderr what data_received() and eof_received() are for stdout. (After all, the point of a subprocess is that "normal" data goes to stdout.) There's only one input stream to the subprocess, so there's no ambiguity for write(), and neither is there a need for multiple connection_made()/lost() methods. (However, we could argue endlessly over whether connection_lost() should be called when the subprocess exits, or when the other side of all three pipes is closed. :-)
Hm. Not excited. I like everyone using the same names for these callback methods, so that a reader (who is familiar with the transport/protocol API) can instantly know what kind of callback it is and what its arguments are. (But see Nick's simple solution for having your cake and eating it, too.)
This is a good observation -- one that I've made myself as well. I also have a plan for dealing with it -- but I haven't coded it up properly yet and consequently I haven't written it up for the PEP yet either. The idea is that there will be some even-higher-level functions for tasks to call to open connections (etc.) which just give you two unidrectional streams (one for reading, one for writing). The write-stream can just be the transport (its write() and writelines() methods are familiar from regular I/O streams) and the read-stream can be a StreamReader -- a class I've written but which needs to be moved into a better place: http://code.google.com/p/tulip/source/browse/tulip/http_client.py#37 Anyway, the reason for having the transport/protocol abstractions in the middle is so that other frameworks can ignore coroutines if they want to -- all they have to do is work with Futures, which can be fully controlled through callbacks (which are native at the lowest level of almost all frameworks, including Tulip / PEP 3156).
It'll be in the stdlib, no worries. I don't expect the overhead to be a problem.
You can write an alternative framework using coroutines and callbacks, bypassing transports and protocols. (You'll still need Futures.) However you'd be missing the interoperability offered by the protocol/transport abstractions: in an IOCP world you'd have to interact with the event loop's callbacks differently than in a select/poll/etc. world. PEP 3156 is trying to make different groups happy: people who like callbacks, people who like coroutines; people who like UNIX, people who like Windows. Everybody may have to compromise a little bit, but the reward will (hopefully) be better portability and better interoperability. -- --Guido van Rossum (python.org/~guido)
On 18 January 2013 22:15, Guido van Rossum <guido@python.org> wrote:
While I don't really care about arguing over *when* connection_lost should be called, it *is* relevant to my thinking that getting notified when the process exits doesn't seem to me to be possible - again it's the issue that the transport can't ask the event loop to poll for anything that the event loop isn't already coded to check. So (once again, unless I've missed something) the only viable option for a standalone transport is to call connection_lost when all the pipes are closed. Am I still missing something? Paul
On Fri, Jan 18, 2013 at 2:57 PM, Paul Moore <p.f.moore@gmail.com> wrote:
That is typically how these things are done (e.g. popen and subprocess work this way). It is also probably the most useful, since it is *possible* that the parent process forks a child and then exits itself, where the child does all the work of the pipeline.
Am I still missing something?
I believe it is, at least in theory, possible to implement waiting for the process to exit, using signals. The event loop can add signal handlers, and there is a signal that gets sent upon child process exit. There are lots of problems here (what if some other piece of code forked that process) but we could come up with reasonable solutions for these. However waiting for the pipes closing makes the most sense, so no need to bother. :-) -- --Guido van Rossum (python.org/~guido)
On 18Jan2013 15:01, Guido van Rossum <guido@python.org> wrote: | It is also probably the most useful, since it is | *possible* that the parent process forks a child and then exits | itself, where the child does all the work of the pipeline. For me, even common. I often make grandchildren instead of children when only the I/O matters so that I don't leave zombies around, nor spurious processes to interfere with wait calls. -- Cameron Simpson <cs@zip.com.au> To have no errors Would be life without meaning No struggle, no joy - Haiku Error Messages http://www.salonmagazine.com/21st/chal/1998/02/10chal2.html
Guido van Rossum wrote:
You don't seem to follow this philosophy anywhere else in the PEP, though. In all the other places a callback is specified, you get to pass in an arbitrary function. The PEP offers no rationale as to why transports should be the odd one out.
I was hoping there would be a slightly higher-level layer, that provides a coroutine interface but hides the platform differences. What would you think of the idea of making the Transport objects themselves fill both roles, by having read_async and write_async methods? They wouldn't have to do any buffering, I'd be happy to wrap another object around it if I wanted that. -- Greg
On Fri, Jan 18, 2013 at 4:42 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Well, yes, it *is* the odd one (or two, counting start_serving()) out. That's because it is the high-level API.
Hm, Transports+Protocols *is* the higher level layer.
You could code that up very simply using sock_recv() and sock_sendall(). But everyone who's thought about performance of select/poll/etc., seems to think that that is not a good model because it will cause many extra calls to add/remove reader/writer. -- --Guido van Rossum (python.org/~guido)
On Fri, Jan 18, 2013 at 5:15 PM, Guido van Rossum <guido@python.org> wrote:
Using separate methods for stderr breaks compatibility with existing Protocols for no good reason (UDP needs a different protocol interface because individual datagrams can't be concatenated; that doesn't apply here since pipes are stream-oriented). We'll have intermediate Protocol classes like LineReceiver that work with sockets; why should they be reimplemented for stderr? It's also likely that if I do care about both stdout and stderr, I'm going to take stdout as a blob and redirect it to a file, but I'll want to read stderr with a line-oriented protocol to get error messages, so I don't think we want to favor stdout over stderr in the interface. I think we should have a pipe-based Transport and the subprocess should just contain several of these transports (depending on which fds the caller cares about; in my experience I rarely have more than one pipe per subprocess, but whether that pipe is stdout or stderr varies). The process object itself should also be able to run a callback when the child exits; waiting for the standard streams to close is sufficient in most cases but not always. -Ben
On Mon, Jan 21, 2013 at 1:23 PM, Ben Darnell <ben@bendarnell.com> wrote:
This is a good point.
That all depends rather on the application.
Unfortunately you'll also need a separate protocol for each transport, since the transport calls methods with fixed names on the protocol (and you've just argued that that we should stick to that -- and I agree :-). Note that since there's (normally) only one input file to the subprocess, only one of these transports should have a write() method -- but both of them have to call data_received() and potentially eof_received() on different objects. And in this case it doesn't seem easy to use the StreamReader class, since you can't know which of the two (stdout or stderr) will have data available first, and guessing wrong might cause a deadlock. (So, yes, this is a case where coroutines are less convenient than callbacks.) -- --Guido van Rossum (python.org/~guido)
On Mon, Jan 21, 2013 at 9:31 PM, Guido van Rossum <guido@python.org> wrote:
Exactly.
Well, to be precise I was arguing that pipe transports should work the same way as socket transports. I'm still not a fan of the use of fixed method names. (As an alternative, what if protocols were just callables that took a Future argument? for data_received future.result() would return the data and for eof_received and connection_lost it would raise an appropriate exception type. That just leaves connection_made, which I was arguing in the other thread should be on the protocol factory instead of the protocol.)
I'd actually give stdin its own transport and protocol, distinct from stdout and stderr (remember that using all three pipes on the same process is relatively uncommon). It's a degenerate case since it will never call data_received, but it's analogous to the way that subprocess uses three read-only and write-only file objects instead of trying to glue stdin and stdout together. This is fairly new and little-tested, but it shows the interface I have in mind: http://tornado.readthedocs.org/en/latest/process.html#tornado.process.Subpro...
I'm not sure I follow. Couldn't you just attach a StreamReader to each stream and use as_completed to read from them both in parallel? You'd get in trouble if one of the streams has a line longer than the StreamReader's buffer size, but that sort of peril is everywhere if you're using both stdout and stderr, no matter what the interface is (unless you just use a large or unlimited buffer and hope you won't run out of memory, like subprocess.communicate). At least with "yield from stderr_stream.readline()" you're better off than with a synchronous subprocess since the StreamReader's buffer size is adjustable, unlike the pipe buffer size. -Ben
-- --Guido van Rossum (python.org/~guido)
On Thu, Jan 17, 2013 at 2:40 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Shouldn't this be called create_internet_transport or something like that?
I just renamed it to create_connection(), like I've been promising for a long time. -- --Guido van Rossum (python.org/~guido)
On Thu, Jan 17, 2013 at 8:59 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Basically yes, in this context. The same assumption underlies socket.getaddrinfo() in the stdlib. If you have a CORBA system lying around and you want to support it, you're welcome to create the transport connection function create_corba_connection(). :-) -- --Guido van Rossum (python.org/~guido)
On Fri, Jan 18, 2013 at 6:08 PM, Paul Moore <p.f.moore@gmail.com> wrote:
I'm not sure why CORBA would be a transport in its own right rather than a protocol running over a standard socket transport. Transports are about the communications channel - network sockets - OS pipes - shared memory - CANbus - protocol tunneling Transports should only be platform specific at the base layer where they actually need to interact with the OS through the event loop. Higher level transports should be connected to lower level protocols based on APIs provided by those transports and protocols themselves. The *whole point* of the protocol vs transport model is to allow you to write adaptive stacks. To use the example from PEP 3153, to implement full JSON-RPC support over both sockets and a HTTP-tunnel you need the following implemented: - TCP socket transport - HTTP protocol - HTTP-based transport - JSON-RPC protocol Because the transport API is standardised, the JSON-RPC protocol can be written once and run over HTTP using the full stack as shown, *or* directly over TCP by stripping out the two middle layers. The *only* layer that the event loop needs to concern itself with is the base transport layer - it doesn't care how many layers of protocols or protocol-as-transport adapters you stack on top. The other thing that may not have been emphasised sufficiently is that the *protocol* APIs is completely dependent on the protocol involved. The API of a pipe protocol is not that of HTTP or CORBA or JSON-RPC or XML-RPC. That's why tunneling, as in the example above, requires a protocol-specific adapter to translate from the protocol API back to the standard transport API. So, for example, Greg's request for the ability to pass callbacks rather than needing particular method names can be satisfied by writing a simple callback protocol: class CallbackProtocol: """Invoke arbitrary callbacks in response to transport events""" def __init__(self, on_data, on_conn, on_loss, on_eof): self.on_data = on_data self.on_conn = on_conn self.on_loss = on_loss self.on_eof = on_eof def connection_made(transport): self.on_conn(transport) def data_received(data): self.on_data(data) def eof_received(): self.on_eof() def connection_lost(exc): self.on_loss(exc) Similarly, his request for a IOStreamProtocol would likely look a lot like an asynchronous version of the existing IO stack API (to handle encoding, buffering, etc), with the lowest layer being built on the transport API rather than the file API (as it is in the io module). You would then be able to treat *any* transport, whether it's an SSH tunnel, an ordinary socket connection or a pipe to a subprocess as a non-seekable stream. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 18 January 2013 08:38, Nick Coghlan <ncoghlan@gmail.com> wrote:
Interesting. On that basis, the whole subprocess interaction scenario is not a low level transport at all (contrary to what I understood from Guido's suggestion of an event loop method) and so should be built in user code (OK, probably as a standard library helper, but definitely not as specialist methods on the event loop) layered on the low-level pipe transport. That was my original instinct, but it fell afoul of 1. The Windows implementation of a low level pipe transport doesn't exist (yet) and I don't know enough about IOCP to write it [1]. 2. I don't understand the programming model well enough to understand how to write a transport/protocol layer (coroutine head explosion issue). I have now (finally!) got Guido's point that implementing a process protocol will give me a good insight into how this stuff is meant to work. I'm still struggling to understand why he thinks it needs a dedicated method on the event loop, rather than being a higher-level layer like you're suggesting, but I'm at least starting to understand what questions to ask. Paul [1] There is some stuff in the IOCP documentation about handles having to be opened in OVERLAPPED mode, which worries me here as it may imply that arbitrary pipes (such as the ones subprocess.Popen uses) can't be plugged in. It's a bit like setting a filehandle to nonblocking in Unix, but it has to be done at open time, IIUC. I think I saw an email about this that I need to hunt out.
On Fri, Jan 18, 2013 at 7:33 PM, Paul Moore <p.f.moore@gmail.com> wrote:
The creation of the pipe transport needs to be on the event loop, precisely because of cross-platform differences when it comes to Windows. On *nix, on the other hand, the pipe transport should look an awful lot like the socket transport and thus be able to use the existing file descriptor based interfaces on the event loop. The protocol part is then about adapting the transport API to coroutine friendly readlines/writelines API (the part that Guido points out needs more detail in http://www.python.org/dev/peps/pep-3156/#coroutines-and-protocols) As a rough untested sketch (the buffering here could likely be a lot smarter): # Remember we're not using preemptive threading, so we don't need locking for thread safety # Note that the protocol isn't designed to support reconnection - a new connection means # a new protocol instance. The create_* APIs on the event loop accept a protocol factory # specifically in order to encourage this approach class SimpleStreamingProtocol: def __init__(self): self._transport = None self._data = bytearray() self._pending = None def connection_made(self, transport): self._transport = transport def connection_lost(self, exc): self._transport = None # Could also store the exc directly on the protocol and raise # it in subsequent write calls if self._pending is not None: self._pending.set_exception(exc) def received_eof(self): self.transport = None if self._pending is not None: self._pending.set_result(False) def received_data(self, data): self.data.extend(data) if self._pending is not None: self._pending.set_result(True) # The writing side is fairly easy, as we just pass it through to the transport # These are all defined by PEP 3156 as non-blocking calls def write(self, data): if self._transport is None: raise RuntimeError("Connection not open") self._transport.write(data) def writelines(self, iterable): if self._transport is None: raise RuntimeError("Connection not open") self._transport.writelines(iterable) def close(self): if self._transport is not None: self._transport.close() self._transport = None def _read_from_buffer(self): data = bytes(self._data) self._data.clear() return data # The reading side has to adapt between coroutines and callbacks @coroutine def read(self): if self._transport is None: raise RuntimeError("Connection not open") if self._pending is not None: raise RuntimeError("Concurrent reads not permitted") # First check if we already have data waiting data = self._read_from_buffer() if data: return data # Otherwise wait for data # This method can easily be updated to use a loop and multiple # futures in order to support a "minimum read" parameter f = self._pending = tulip.Future() finished = yield from f data = b'' if finished else self._read_from_buffer() return data # This uses async iteration as described at [1] # We yield coroutines, which must then be invoked with yield from def readlines(self): cached_lines = self._data.split(b'\n') self._data.clear() if cached_lines[-1]: # Last line is incomplete self._data.extend(cached_lines[-1]) del cached_lines[-1] while not finished: # When we already have the data, a simple future will do for line in cached_lines: f = tulip.Future() f.set_result(line) yield f # Otherwise, we hand control to the event loop @coroutine def wait_for_line(): nonlocal finished data = yield from self.read() if not data: finished = True return b'' lines = data.split(b'\n') if lines[-1]: # Last line is incomplete self._data.extend(lines[-1]) cached_lines.extend(lines[1:-1]) return lines[0] yield wait_for_line() # Used as: pipe, stream = event_loop.create_pipe(SimpleStreamingProtocol) # Or even as: conn, stream = event_loop.create_connection(SimpleStreamingProtocol, ... # connection details) # Reading from the stream in a coroutine for f in stream.readlines(): line = yield from f [1] http://python-notes.boredomandlaziness.org/en/latest/pep_ideas/async_program... Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Fri, Jan 18, 2013 at 3:55 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Thanks for clarifying that -- I'm behind on this thread!
I have a more-or-less working but probably incomplete version checked into the tulip repo: http://code.google.com/p/tulip/source/browse/tulip/subprocess_transport.py Note that this completely ignores stderr -- this makes the code simpler while still useful (there's plenty of useful stuff you can do without reading stderr), and avoids the questions Greg Ewing brought up about needing two transports (one for stdout, another for stderr). -- --Guido van Rossum (python.org/~guido)
On 18 January 2013 22:22, Guido van Rossum <guido@python.org> wrote:
Ha! You beat me to it. OK, looking at your code, I see that you freely used the add_reader/add_writer functions and friends, and the fact that the Unix selectors handle pipes as well as sockets. With the freedom to do that, your code looks both reasonable and pretty straightforward. I was having trouble getting past the fact that this approach wouldn't work on Windows, and confusing "nonportable" with "not allowed". My apologies. You kept telling me that writing the code for Unix would be helpful, but I kept thinking in terms of writing code that worked on Unix but with portability to Windows in mind, which completely misses the point. I knew that the transport/protocol code I'd end up writing would look something like this, but TBH I'd not seen that as the interesting part of the problem... BTW, to avoid duplication of the fork/exec stuff, I would probably have written the transport to take a subprocess.Popen object as its only argument, then hooked up self._wstdin to popen.stdin and self._rstdout to popen.stdout. That requires the user to have created the Popen object with those file descriptors as pipes (I don't know if it's possible to introspect a Popen object to check that) but avoids duplicating the subprocess logic. I can probably fairly quickly modify your code to demonstrate, but it's late and I don't want to start booting my Unix environment now, so it'll have to wait till tomorrow :-) Paul.
On Fri, Jan 18, 2013 at 2:48 PM, Paul Moore <p.f.moore@gmail.com> wrote:
Glad you've got it now!
I would love for you to create that version. I only checked it in so I could point to it -- I am not happy with either the implementation, the API spec, or the unit test... -- --Guido van Rossum (python.org/~guido)
On 18 January 2013 22:53, Guido van Rossum <guido@python.org> wrote:
May be a few days before I can get to it. Apparently when Ubuntu installs an automatic upgrade, it feels that it's OK to break the wireless drivers. I now have the choice of scouring the internet on another PC to find possible solutions (so far that approach is a waste of time...), or reinstalling the OS. How do you Linux users put up with this sort of thing? :-) Seriously, I'm probably going to have to build a VM so I don't get this sort of unnecessary hardware issue holding me up. Paul
On 19 January 2013 12:12, Paul Moore <p.f.moore@gmail.com> wrote:
OK, I finally have a working VM. The subprocess test code assumes that it can call transport.write_eof() in the protocol connection_made() function. I'm not sure if that is fundamental, or just an artifact of the current implementation. Certainly if you have a stdin pipe open, you likely want to close it to avoid deadlocks, but with the subprocess.Popen approach, it's entirely possible to not open a pipe to stdin. In that case, writing to stdin is neither possible nor necessary. Clearly, writing data to stdin if you didn't open a pipe should be flagged as an error. And my immediate thought is that write_eof should also be an error. But I can imagine people wanting to write reusable protocols that pre-emptively write EOF to the stdin pipe to avoid deadlocks. So, a question: If the user passed a popen object without a stdin pipe, should write_eof be an error or should it just silently do nothing? Paul
On Tue, Jan 22, 2013 at 10:03 PM, Paul Moore <p.f.moore@gmail.com> wrote:
It should be an error. The analogy is similar to calling flush() vs close(). Calling flush() on an already closed file is an error, while you can call close() as many times as you like. If you want to ensure a pipe is closed gracefully, call close(), not write_eof(). (abort() is the method for abrupt closure). Also, I agree with the comment someone else made that attempting to pair stdin with either stderr or stdout is a bad idea - better to treat them as three independent transports (as the subprocess module does), so that the close() semantics and error handling are clear. sockets are different, as those actually *are* bidirectional data streams, whereas pipes are unidirectional. I don't know whether it's worth defining separate SimplexTransmit (e.g. stdin pipe in parent process, stdout, stderr pipes in child process), SimplexReceive (stdout, stderr pipes in parent process, stdin pip in child process), HalfDuplex (e.g. some radio transmitters) and FullDuplex (e.g. sockets) transport abstractions - I guess if Twisted haven't needed them, it probably isn't worth bothering. It's also fairly obvious how to implement the first three based on the full duplex API currently described in the PEP just be raising the appropriate exceptions. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 22 January 2013 13:34, Nick Coghlan <ncoghlan@gmail.com> wrote:
That was my original feeling - although I made my case badly by arguing in terms of portability rather than clearer design. But Guido argued for a higher-level portable subprocess transport that was implemented "under the hood" using the existing nonportable add_reader/add_writer methods on Unix, and an as-yet-unimplemented IOCP-based alternative on Windows. I still feel that a more general approach would be to have two methods on the event loop connect_input_pipe(protocol_factory, readable_pipe) and connect_output_pipe(protocol_factory, writeable_pipe) which use the standard transport/protocol methods as defined in the PEP. Then the subprocess transport can be layered on top of that as one possible example of a "higher layer" convenience transport. I know that twisted has a create_process event loop (reactor) method, but I suspect part of the reason for that is that it predates the subprocess module's unified interface. I'll try implementing the pipe transport approach and see how it looks in contrast. Paul.
On 22 January 2013 15:43, Paul Moore <p.f.moore@gmail.com> wrote:
I'll try implementing the pipe transport approach and see how it looks in contrast.
Here's a quick proof of concept (for a read pipe): class UnixEventLoop(events.EventLoop): ... @tasks.task def connect_read_pipe(self, protocol_factory, rpipe): protocol = protocol_factory() waiter = futures.Future() transport = _UnixReadPipeTransport(self, rpipe, protocol, waiter) yield from waiter return transport, protocol class _UnixReadPipeTransport(transports.Transport): def __init__(self, event_loop, rpipe, protocol, waiter=None): self._event_loop = event_loop self._pipe = rpipe.fileno() self._protocol = protocol self._buffer = [] self._event_loop.add_reader(self._pipe.fileno(), self._read_ready) self._event_loop.call_soon(self._protocol.connection_made, self) if waiter is not None: self._event_loop.call_soon(waiter.set_result, None) def _read_ready(self): try: data = os.read(self._pipe, 16*1024) except BlockingIOError: return if data: self._event_loop.call_soon(self._protocol.data_received, data) else: self._event_loop.remove_reader(self._pipe) self._event_loop.call_soon(self._protocol.eof_received) Using this to re-implement the subprocess test looks something like this (the protocol is unchanged from the existing test): def testUnixSubprocessWithPipe(self): proc = subprocess.Popen(['/bin/ls', '-lR'], stdout=subprocess.PIPE) t, p = yield from self.event_loop.connect_read_pipe(MyProto, proc.stdout) self.event_loop.run() To be honest, this looks sufficiently straightforward that I don't see the benefit in a less-general high-level transport type... Paul
I am not actually very committed to a particular design for a subprocess transport. I'll happily leave it up to others to come up with a design and make it work on multiple platforms. --Guido van Rossum (sent from Android phone)
On 22 January 2013 16:33, Guido van Rossum <guido@python.org> wrote:
OK. I've written a pipe transport (event_loop.connect_read_pipe and event_loop.connect_write_pipe) and modified the existing subprocess test to use it. I've also added a small read/write test. The code is in my bitbucket repository at https://bitbucket.org/pmoore/tulip. I'm not very happy with the call-back based style of the read/write test. I'm sure it would be much better written in an async style, but I don't know how to do so. If anyone who understands the async style better than I do can offer a translation, I'd be very grateful - I'd like to see if the resulting code looks sufficiently clear. Here's the relevant code. The biggest ugliness is the need for the two protocol classes, which basically do nothing but (1) collect data received and (2) ignore unwanted callbacks. class DummyProto(protocols.Protocol): def __init__(self): pass def connection_made(self): pass def data_received(self, data): pass def eof_received(self): pass def connection_lost(): pass class MyCollector(protocols.Protocol): def __init__(self): self.data = [] def connection_made(self): pass def data_received(self, data): self.data.append(data) def eof_received(self): pass def connection_lost(): pass def get_data(self): return b''.join(self.data) def testReadWrite(self): proc = Popen(['/bin/tr', 'a-z', 'A-Z'], stdin=PIPE, stdout=PIPE) rt, rp = yield from self.event_loop.connect_read_pipe(MyCollector, proc.stdout) wt, wp = yield from self.event_loop.connect_read_pipe(DummyProto, proc.stdin) def send_data(): wt.write("hello, world") wt.write_eof() self.event_loop.call_soon(send_data) self.event_loop.run() self.assertEqual(rp.get_data(), b'HELLO, WORLD') Paul
Guido van Rossum wrote: […]
Although 3 pipes to a subprocess (stdin, stdout, stderr) is the usual convention, it's not the only possibility, so that configuration shouldn't be hard-coded. On POSIX some programs can and do make use of the ability to have more pipes to a subprocess; e.g. the various *fd options of gnupg (--status-fd, --logger-fd, --command-fd, and so on). And some programs give the child process file descriptors that aren't pipes, like sockets (e.g. an inetd-like server that accepts a socket then spawns a subprocess to serve it). So I hope tulip will support these possibilities (although obviously the stdin/out/err style should be the convenient default). You will be unsurprised to hear that Twisted does :) (Please forgive me if this was already pointed out. It's hard keeping up with python-ideas.) -Andrew.
On 20 January 2013 22:53, Andrew Bennetts <andrew@bemusement.org> wrote:
My plan is to modify Guido's current code to take a subprocess.Popen object when creating a connection to a subprocess. So you'd use the existing API to start the process, and then tulip to interact with it. Having said that, I have no idea if or how subprocess.Popen would support the extra fds you are talking about. If you can show me some sample code, I can see what would be needed to handle it. But as far as I know, subprocess.Popen objects only have the 3 standard handles exposed as attributes - stdin, stdout and stderr. If you have to create your own pipes and manage them yourself in "normal" code, then I would expect that you'd have to do the same with tulip. That may indicate a need for (yet another) event loop API to create a pipe which can then be used with subprocess. Or you could use the add_reader/add_writer interfaces, at the expense of portability. Paul PS The above is still my plan. But at the moment, every PC in my house seems to have decided to stop working, so I'm rebuilding PCs rather than doing anything useful :-( Normal service will be resumed in due course...
On Sun, Jan 20, 2013 at 3:25 PM, Paul Moore <p.f.moore@gmail.com> wrote:
subprocess.Popen has the pass_fds argument, documented as follows: *pass_fds* is an optional sequence of file descriptors to keep open between the parent and child. Providing any *pass_fds* forces *close_fds*to be True <http://docs.python.org/dev/library/constants.html#True>. (Unix only) Eli
On 20 January 2013 23:29, Eli Bendersky <eliben@gmail.com> wrote:
I thought that was the case, but it seems like this is only really enabling you to manually manage the extra pipes as I was suggesting in my comment. My current expectation is that the API would be something like eventloop.connect_process(protocol_factory, popen_obj) and the protocol would have data_received and err_received methods called when the stdout or stderr fds have data, and the transport would have a write method to write to stdin. If anyone has a suggestion for an API that could be used for arbitrary FDs (which I presume could be either input or output) on top of this, I'd be happy to incorporate it - but personally, I can't think of anything that wouldn't be unusably complex :-( Paul
On Sun, Jan 20, 2013 at 2:53 PM, Andrew Bennetts <andrew@bemusement.org> wrote:
Hm. I agree that something to represent an arbitrary pipe or pair of pipes may be useful occasionally, and we need to have an implementation that can deal with stdout and stderr separately anyway, but I don't think such extended configurations are common enough that we need to completely generalize the API. I think it is fine to follow the example of subprocess.py, which allows but does not encourage extra pipes and treats stdin, stdout and stderr differently.
-- --Guido van Rossum (python.org/~guido)
On 18 January 2013 09:33, Paul Moore <p.f.moore@gmail.com> wrote:
Hmm, I'm looking at a pipe transport on Unix, and I find I don't know enough about programming Unix. How do I set a file descriptor (specifically a pipe) in Unix to be nonblocking? For a socket, sock.setblocking(False) does the job. But for a pipe/file, the only thing I can see is the O_NONBLOCK flag to os.open/os.pipe2. Is it not possible to set an already open file descriptor to be nonblocking? If that's the case, it means that Unix has the same problem as I suspect exists for Windows - existing pipes and filehandles can't be used in async code as they won't necessarily be in nonblocking mode. Is there a way round this on Unix that I'm not aware of? Otherwise, it seems that there's going to have to be a whole load of duplication in the "async world" (an async version of subprocess.Popen, for a start, as well as any other "open" type of calls that might need to produce handles that can be used asynchronously). Either that or everything that returns a pipe/handle that you might want to use in async code will have to grow some sort of "async" flag. Paul
On Fri, Jan 18, 2013 at 09:01:32PM +0000, Paul Moore <p.f.moore@gmail.com> wrote:
http://linuxmanpages.com/man2/fcntl.2.php The file status flags A file descriptor has certain associated flags, initialized by open(2) and possibly modified by fcntl(2). The flags are shared between copies (made with dup(2), fork(2), etc.) of the same file descriptor. The flags and their semantics are described in open(2). F_GETFL Read the file descriptor's flags. F_SETFL Set the file status flags part of the descriptor's flags to the value specified by arg. Remaining bits (access mode, file creation flags) in arg are ignored. On Linux this command can only change the O_APPEND, O_NONBLOCK, O_ASYNC, and O_DIRECT flags. Oleg. -- Oleg Broytman http://phdru.name/ phd@phdru.name Programmers don't die, they just GOSUB without RETURN.
On Sat, Jan 19, 2013 at 01:25:31AM +0400, Oleg Broytman <phd@phdru.name> wrote:
So you have to call fnctl() on the pipe's descriptor to F_GETFL flags, set O_NONBLOCK and call fnctl() to F_SETFL the new flags back. Oleg. -- Oleg Broytman http://phdru.name/ phd@phdru.name Programmers don't die, they just GOSUB without RETURN.
Paul Moore wrote:
No, it doesn't -- a fd doesn't *have* to be non-blocking in order to use it with select/poll/whatever. Sometimes people do, but only to allow a performance optimisation by attempting another read before going back to the event loop, just in case more data came in while you were processing the first lot. But doing that is entirely optional. Having said that, fcntl() is usually the way to change the O_NONBLOCK flag of an already-opened fd, althought the details may vary from one unix to another. -- Greg
On Fri, Jan 18, 2013 at 12:38 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
I don't know -- but I could imagine that a particular CORBA implementation might be provided as a set of API function calls rather than something that hooks into sockets. I don't care about CORBA, but that was the use case I intended to highlight -- something that (for whatever reason, no matter how misguided) doesn't use sockets and doesn't have an underlying file descriptor you can wait on. (IIRC most GUI frameworks also fall into that category.)
Hm. I think of transports more as an abstraction of a specific set of semantics for a communication channel -- bidrectional streams, in particular, presumably with error correction/detection so that you can assume that you either see what the other end sent you, in the order in which it sent it (but not preserving buffer/packet/record boundaries!), or you get a "broken connection" error. Now, we may be in violent agreement here -- the transports I am thinking of can certainly use any of the mechanisms you list as underlying abstraction. But I wouldn't call it a transport unless it had standardized semantics and a standardized interface with the protocol. (For datagrams, we need slightly different abstractions, with different guarantees and semantics. But, again, all datagram transports should be more or less interchangeable.)
Yeah, well, but in practice I expect that layering transports on top of each other is rare, and using platform specific transport implementations is by far the common case. (Note that in theory you could layer SSL over any unencrypted transport; but in practice (a) few people need that, and (b) the ssl module doesn't support this -- hence I am comfortable with treating SSL as another platform-specific transport.)
I don't know enough about JSON-RPC (shame on me!) but this sounds very reasonable.
True. There's one important issue here: *constructing* the stack is not up to the event loop. It is totally fine if the HTTP-based transport is a 3rd party package that exports a function to set up the stack, given an event loop and a protocol to run on top (JSON-RPC in this example). This function can have a custom signature that is not compatible with any other transport-creating APIs in existence. (In fact this is why I renamed create_transport() to create_connection() -- the standardized API just has methods for creating internet connections.)
I'm not even sure what you mean by the protocol API. From the PEP's POV, the "protcol API" is just the methods that the transport calls (connection_made(), data_received(), etc.) and those certainly *are* supposed to be standardized.
So, for example, Greg's request for the ability to pass callbacks rather than needing particular method names
Hm, I have yet to respond to Greg's message, but I'm not sure that's a reasonable request.
Well, except that you can't just pass CallbackProtocol where a protocol factory is required by the PEP -- you'll have to pass a lambda or partial function without arguments that calls CallbackProtocol with some arguments taken from elsewhere. No big deal though.
That sounds like an intriguing idea which I'd like to explore in the distant future. One point of light: a transport probably already is acceptable as a binary *output* stream, because its write() method is not a coroutine. (This is intentional.) But doing the same for input is harder.
Right. (TBH, I'm often not sure whether you are just explaining the PEP's philosophy or trying to propose changes... Sorry for the confusion this may cause.) -- --Guido van Rossum (python.org/~guido)
Guido van Rossum wrote:
Something smells wrong to me about APIs that require protocol factories. I don't see what advantage there is in writing create_connection(HTTPProtocol, "some.where.net", 80) as opposed to just writing something like HTTPProtocol(TCPTransport("some.where.net", 80)) You're going to have to use the latter style anyway to set up anything other than the very simplist configurations, e.g. your earlier 4-layer protocol stack example. So create_connection() can't be anything more than a convenience function, and unless I'm missing something, it hardly seems to add enough convenience to be worth the bother. -- Greg
On Fri, Jan 18, 2013 at 3:59 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
Glyph should really answer this one. Personally I don't feel strongly either way for this case. There may be an advantage to not calling the protocol factory if the connection can't be made (in which case the Future returned by create_connection() has the exception). -- --Guido van Rossum (python.org/~guido)
On Fri, 18 Jan 2013 16:12:29 -0800 Guido van Rossum <guido@python.org> wrote:
Except that you probably want the protocol to outlive the transport if you want to deal with reconnections or connection failures, and therefore: TCPClient(HTTPProtocol(), ("some.where.net", 80)) Regards Antoine.
Antoine Pitrou wrote:
I don't see how to generalise that to more complicated protocol stacks, though. For dealing with re-connections, it seems like both the protocol *and* the transport need to outlive the connection failure, and the transport needs a reconnect() method that is called by a protocol that can deal with that situation. Reconnection can then propagate along the whole chain. -- Greg
Just like there's no reason for having a protocol without a transport, it seems like there's no reason for a transport without a connection, and that separating the two might further normalize differences between client and server channels Shane Green www.umbrellacode.com 805-452-9666 | shane@umbrellacode.com On Jan 18, 2013, at 8:16 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
On Jan 18, 2013, at 4:12 PM, Guido van Rossum <guido@python.org> wrote:
Glyph should really answer this one.
Thanks for pointing it out to me, keeping up with python-ideas is always a challenge :).
On Fri, Jan 18, 2013 at 3:59 PM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
For starters, nothing "smells wrong" to me about protocol factories. Responding to this kind of criticism is difficult, because it's not substantive - what's the actual problem? I think that some Python programmers have an aversion to factories because a common path to Python is flight from Java environments that over- or mis-use the factory pattern.
Guido mentioned one advantage already; you don't have to create the protocol object if the connection fails, so your protocol objects are real honest-to-goodness connections, not "well, maybe there's a connection or maybe there'll be a connection later". To be fair, this is rarely of practical utility, but in edge cases where you are doing something like, "simultaneously try to connect to these 1000 hosts, and give up on all outstanding connections when the first 3 connections succeed", being able to avoid all the construction overhead for your protocols if they're not going to be used is nice. There's a more pressing issue of correctness though: even if you create the protocol in advance, you really don't want to tell it about the transport until the transport truly exists. The connection to some.where.net (by which I mean, ahem, "somewhere.example.com"; "where.net" will not thank you if you ignore BCP 32 in the documentation or examples) might fail, and if the client wants to issue a client greeting, it should not have access to its half-formed transport before that failure. Of course, it's possible to present an API that works around this by buffering writes issued before the connection is established, and by the protocol waiting for the connection_made callback before actually doing its work. Finally, using a factory also makes client-creating and server-creating code more symmetrical, since you clearly need a protocol factory in the listening-socket case. If your main example protocol is HTTP, this doesn't make sense*, but once you start trying to do things like SIP or XMPP, where the participants in a connection are really peers, having the structure be similar is handy. In the implementation, it's nice to have things set up this way so that the order of the protocol<->transport symmetric setup is less important and by the time the appropriate methods are being invoked, everybody knows about everybody else. The transport can't really have a reference to the protocol in the protocol's constructor. *: Unless you're doing this, of course <http://wiki.secondlife.com/wiki/Reverse_HTTP>. However, aside from the factory-or-not issue, the fact that TCPTransport's name implies that it is both (1) a class and (2) the actual transport implementation, is more problematic. TCPTransport will need multiple backends for different multiplexing and I/O mechanisms. This is why I keep bringing up IOCP; this is a major API where the transport implementation is actually quite different. In Twisted, they're entirely different classes. They could probably share a bit more implementation than they do and reduce a little duplication, but it's nice that they don't have to. You don't want to burden application code with picking the right one, and it's ugly to smash the socket-implementation-selection into a class. (create_connection really ought to be a method on an event-loop object of some kind, which produces the appropriate implementation. I think right now it implicitly looks it up in thread-local storage for the "current" main loop, and I'd rather it were more explicit, but the idea is the same.) Your example is misleadingly named; surely you mean TCPClient, because a TCPTransport would implicitly support both clients and servers - and a server would start with a socket returned from accept(), not a host and port. (Certainly not a DNS host name.) create_connection will actually need to create multiple sockets internally. See <http://tools.ietf.org/html/rfc3493> covers this, in part (for a more condensed discussion, see <https://twistedmatrix.com/trac/ticket/4859>).
I don't see how this is true. I've written layered protocols over and over again in Twisted and never wanted to manually construct the bottom transport for that reason.* In fact, the more elaborate multi-layered structures you have to construct when a protocol finishes connecting, the more you want to avoid being required to do it in advance of actually needing the protocols to exist. *: I _have_ had to manually construct transports to deal with some fiddly performance-tuning issues, but those are just deficiencies in the existing transport implementation that ought to be remedied.
*Just* implementing the multiple-parallel-connection-attempts algorithm required to deal with the IPv6 transition period would be enough convenience to be worth having a function, even if none of the other stuff I just wrote applied :). -glyph
Glyph wrote:
I'm not averse to using the factory pattern when it genuinely helps. I'm questioning whether it helps enough in this case to be worth using.
I would suggest that merely instantiating a protocol object should be cheap enough that you don't normally care. Any substantive setup work should be done in the connection_made() method, not in __init__(). Transports are already a "maybe there's a connection" kind of deal, otherwise why does connection_made() exist at all?
Which it seems to me is the way *all* protocols should be written. If necessary, you could "encourage" people to write them this way by having a transport refuse to accept any writes until the connection_made() call has occurred.
They don't have to be classes, they could be functions: create_http_protocol(create_tcp_transport("hammerme.seeificare.com", 80)) The important thing is that each function concerns itself with just one step of the chain, and chains of any length can be constructed by composing them in the obvious way.
Maybe. Or maybe the constructor could be called in more than one way -- create_tcp_transport(host, port) on the client side and create_tcp_transport(socket) on the server side.
Couldn't all that be handled inside the transport?
So what does the code for setting up a multi-layer stack look like? How does it make use of create_connection()? Also, what does an implementation of create_connection() look like that avoids creating the protocol until the connection is made? It seems tricky, because the way you know the connection is made is that it calls connection_made() on the protocol. But there's no protocol yet. So you would have to install a temporary protocol whose connection_made() creates the real protocol. That sounds like it could be even more overhead than just creating the real protocol in the first place, as long as the protocol doesn't do any work until its connection_made() is called. -- Greg
On Fri, Jan 18, 2013 at 8:23 PM, Glyph <glyph@twistedmatrix.com> wrote:
I think the smell is that the factory is A) only used once and B) invoked without adding any additional arguments that weren't available when the factory was passed in, so there's no clear reason to defer creation of the protocol. I think it would make more sense if the transport were passed as an argument to the factory (and then we could get rid of connection_made as a required method on Protocol, although libraries or applications that want to separate protocol creation from connection_made could still do so in their own factories). -Ben
On Jan 19, 2013, at 9:32 AM, Ben Darnell <ben@bendarnell.com> wrote:
The problem with creating the protocol with the transport as an argument to its constructor is that in order to behave correctly, the transport needs to know about the protocol as well; so it also wants to be constructed with a reference to the protocol to *its* constructor. So adding a no-protocol-yet case adds more edge-cases to every transport's implementation. All these solutions are roughly isomorphic to each other, so I don't care deeply about it. However, my proposed architecture has been in use for a decade in Twisted without any major problems I can see. I'm not saying that Twisted programs are perfect, but it would *really* be useful to discuss this in terms of problems you can identify with the humungous existing corpus of Twisted-using code, and say "here's a problem that develops in some programs due to the sub-optimal shape of this API". Unnecessary class definitions, for example, or a particular type of bug; something like that. For example, I can identify several difficulties with Twisted's current flow-control setup code and would not recommend that it be copied exactly. Talking about how the code smells or what might hypothetically make more sense is just bikeshedding. -glyph
On Sun, Jan 20, 2013 at 8:53 AM, Glyph <glyph@twistedmatrix.com> wrote:
But the trade-off in separating protocol creation from notification of the connection is that it means every *protocol* has to be written to handle the "no connection yet" gap between __init__ and the call to connection_made. However, if we instead delay the call to the protocol factory until *after the connection is made*, then most protocols can be written assuming they always have a connection (at least until connection_lost is called). A persistent protocol that spanned multiple connect/reconnect cycles could be written such that you passed "my_protocol.connection_made" as the protocol factory, while normal protocols (that last only the length of a single connection) would pass "MyProtocol" directly. At the transport layer, the two states "has a protocol" and "has a connection" could then be collapsed into one - if there is a connection, then there will be a protocol, and vice-versa. This differs from the current status in PEP 3156, where it's possible for a transport to have a protocol without a connection if it calls the protocol factory well before calling connection_made. Now, it may be that *there's a good reason* why conflating "has a protocol" and "has a connection" at the transport layer is a bad idea, and thus we actually *need* the "protocol creation" and "protocol association with a connection" events to be distinct. However, the PEP currently doesn't explain *why* it's necessary to separate the two, hence the confusion for at least Greg, Ben and myself. Given that new protocol implementations should be significantly more common than new transport implementations, there's a strong case to be made for pushing any required complexity into the transports. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Sat, Jan 19, 2013 at 5:51 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
That doesn't strike me as a problematic design. I've seen it plenty of times.
Well, almost. connection_made() would have to return self to make this work. But we could certainly use add some other method that did that. (At first I thought it would be harder to pass other parameters to the constructor for the non-reconnecting case, but the solution is about the same as before -- use a partial function or a lambda that takes a protocol and calls the constructor with that and whatever other parameters it wants to pass.)
This doesn't strike me as important. The code I've written for Tulip puts most of the connection-making code outside the transport, and the transport constructor is completely private. Every transport implementation is completely free in how it works, and every event loop implementation is free to put as much or as little of the connection set-up in the transport as it wants to. The same is true for transports written by users (and there will be some of these). The *only* things we care about for transports is that the thing passed to the protocol's connection_made() has the methods specified by the PEP (write(), writelines(), pause(), resume(), and a few more). Also, it does not matter one iota whether it is the transport or some other entity that calls the protocol's methods (connection_made(), data_received(), etc.) -- the only thing that matters is the order in which they are called. IOW, even though a transport may "have" a protocol without a connection, nobody should care about that state, and nobody should be calling its methods (again, write() etc.) in that state. In fact, nobody except event loop internal code should ever have a reference to a transport in that state. (The transport that is returned by create_connection() is fully connected to the socket (or whatever might takes its place) as well as to the protocol.) I think we can make the same assumptions for transports implemented by user code.
So, your whole point here seems to be that you'd rather see the PEP specify that the sequence when a connection is made is protocol = protocol_factory(transport) rather than protocol = protocol_factory() protocol.connection_made(transport) I looked in the Tulip code to see whether this would cause any problems. I think it could be done, but the solution would feel a little awkward to me, because currently the protocol's connection_made() method is not called directly by the transport: it is called indirectly via the event loop's call_soon() method. So using your approach the transport wouldn't have a protocol attribute until this callback is called -- or we'd have to change things to call it directly rather than via call_soon(). Now I'm pretty sure I can prove that nothing will be referencing the protocol *before* the connection_made() call is actually made, and also that directly calling it instead of using call_soon() is fine. But nevertheless the transport code would feel a little harder to reason about.
TBH I don't see the protocol implementation getting any simpler because of this. There is some protocol initialization code that doesn't depend on the transport, and some that does. Using your approach, these all go in __init__(). Using the PEP's current proposal, the latter go in a separate method, connection_made(). But using your approach, writing the lambda or partial function that calls the constructor with the right arguments (to be passed as protocol_factory) becomes a tad more complex, since now it must take a transport argument. On the third hand, rigging things so that a pre-existing protocol instance can be reused becomes a little harder to figure out, since you have to write a helper method that takes a transport and returns the protocol (i.e., self). All in all I see it as six of one, half a dozen of the other, and I am happy with Glyph's testimony that the Twisted design works well in practice. -- --Guido van Rossum (python.org/~guido)
On Sun, Jan 20, 2013 at 2:35 PM, Guido van Rossum <guido@python.org> wrote:
When the two are separated without a clear definition of what else can happen in between, *every other method on the protocol* needs to cope with the fact that other calls to protocol methods may happen in between the call to __init__ and the call to connection_made - you simply can't write a protocol without dealing with that problem. As you correctly figured out, my specific proposal was to move from: protocol = protocol_factory() protocol.connection_made(transport) To a single event: protocol = protocol_factory(transport) The *reason* I wanted to do this is that I *don't understand* what may happen to my protocol implementation between construction and the call to make_connection. Your description of the current implementation actually worries me, as it suggests to me that when I get a (transport, protocol) pair back from a call to "create_connection", "connection_made" may *not* have been called yet - the protocol may be in exactly the state I am worried about, because the event loop is sending the notification in a fire-and-forget fashion, instead of waiting until the call is complete: protocol = protocol_factory() loop.call_soon(protocol.connection_made, transport) # The protocol isn't actually fully initialized here... However, that description also made me realise why two distinct operations are needed, so I'd like to change my suggestion to the following: protocol = factory() yield from protocol.connection_made(transport) # Or callback equivalent The protocol factory would still be used to create the protocol object. However, the PEP would be updated to make it clear that immediately after creation the *only* permitted method invocation on the result is "connection_made", which will complete the protocol initialization process. The connection_made event handler would be redefined to return a *Future* (or equivalent object) rather than completing synchronously. create_connection would then call connection_made and *wait for it to finish*, rather than using call_soon in a fire-and-forget fashion. The advantage of this is that the rationale for the various possible states become clear: - the protocol factory is invoked synchronously, and is thus not allowed to perform any blocking actions (but may trigger "fire-and-forget" operations) - connection_made is invoked asynchronously, and is thus able to wait for various operations - a protocol returned from create_connection is certain to have had connection_made already called, thus a protocol implementation may safely assume in other methods that both __init__ and connection_made will have been called during the initialization process. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Jan 19, 2013, at 10:13 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Nope. You only have to deal with the methods that the transport will call on the protocol in that state, since nothing else has a reference to it yet. Except the transport won't call them in that state, so... still nope. Again: there's enormous corpus of Twisted code out there that is written this way, you can go look at that code to see how it deals with the problem you've imagined, which is to say: it doesn't. It doesn't need to. Now, if you make the change you're proposing, and tie together the protocol's construction with the transport's construction, so that you end up with protocol(transport(...)), this means that the protocol will immediately begin interacting with the transport in this vague, undefined, not quite connected state, because, since the protocol didn't even exist at the time of the transport's construction, the transport can't possibly have a reference to a protocol yet. And the side of the protocol that issues a greeting will necessarily need to do transport.write(), which may want to trigger a notification to the protocol of some kind (flow control?), and where will that notification go? It needs to be solved less often, but it's a much trickier problem to solve. There are also some potential edge-cases where the existing Twisted-style design might be nicer, like delivering explicit TLS handshake notifications to protocols which support them in the vague state between protocol construction and connection_made, but seeing as how I still haven't gotten around to implementing that properly in Twisted, I imagine it will be another 10 years before Tulip is practically concerned with it :). Finally, I should say that Guido's point about the transport constructor being private is actually somewhat important. We've been saying 'transport(...)' thus far, but in fact it's more like 'SocketTransport(loop, socket)'. Or perhaps in the case of a pipe, 'PipeTransport(loop, readfd, writefd)'. In the case of an actual outbound TCP connection with name resolution, it's 'yield from make_outgoing_tcp_transport(loop, hostname, port)'. Making these all methods that hide the details and timing of the transport's construction is a definite plus. -glyph
On Sun, Jan 20, 2013 at 4:51 PM, Glyph <glyph@twistedmatrix.com> wrote:
Yes, after Guido explained how tulip was currently handling this, I realised that the problem was mostly one of documentation. However, I think there is one key bug in the current implementation, which is that create_connection is returning *before* the call to "connection_made" is completed, thus exposing the protocol in an incompletely initialised state.
Yes, I didn't have a problem with that part - it was just the lack of clear explanation of the different roles of the protocol constructor and the connection_made callback that I found problematic. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Jan 19, 2013, at 11:18 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Aah. Yes, I think you're right about that being a bug. There are probably some docs in Twisted that could be improved to explain that this ordering is part of our analogous interface's contract...
I wasn't clear if you were arguing against it; I just wanted to make it clear :). -glyph
Glyph wrote:
You still haven't explained why the protocol can't simply refrain from doing anything with the transport until its connection_made() is called. If a transport is always to be assumed ready-to-go as soon as it's exposed to the outside world, what is the point of having connection_made() at all? -- Greg
On Sat, 19 Jan 2013 20:35:04 -0800 Guido van Rossum <guido@python.org> wrote:
This is just not true. When the connection breaks, the protocol still has a reference to the transport and may still be trying to do things with the transport (because connection_lost() has not been called yet). Regards Antoine.
On Sun, Jan 20, 2013 at 4:36 AM, Antoine Pitrou <solipsis@pitrou.net> wrote:
That's a different case though. There once *was* a connection. You are right that the transport needs to protect itself against the protocol making further calls to the transport API in this case. Anyway, I think Nick is okay with the separation between the protocol_factory() call and the connection_made() call, as long as the future returned by create_connection() isn't marked done until the connection_made() call returns. That's an easy fix in the current Tulip code. It's a little harder though to fix up the PEP to clarify all this... -- --Guido van Rossum (python.org/~guido)
On Mon, Jan 21, 2013 at 5:03 AM, Guido van Rossum <guido@python.org> wrote:
Right, I understand what the separate method enables now. I think one way to make it clearer in the PEP is to require that "connection_made" return a Future or coroutine, rather than being an ordinary method returning None. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Sun, Jan 20, 2013 at 11:52 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Hm. This would seem to introduce Futures / coroutines at the wrong level (I want to allow protocol implementers to use them, but not require them). If connection_made() wants to initiate some blocking I/O, it is free to do so, but it ought to wrap that in a Task. If the class needs completion of this task to be a prerequisite for handling data passed to a subsequent data_received() call, it will need to devise some buffering and/or locking scheme that's outside the scope of the PEP. Note that I am also hoping to produce a more coroutine-oriented style for writing protocols. The main piece of code for this already exists, the StreamReader class (http://code.google.com/p/tulip/source/browse/tulip/http_client.py?r=b1028ab0...), but I need to think about how to hook it all together nicely (for writing, the transport's API is ready to be used by coroutines). -- --Guido van Rossum (python.org/~guido)
On Fri, Jan 18, 2013 at 12:08 AM, Paul Moore <p.f.moore@gmail.com> wrote:
No, it doesn't need to be a method on the event loop at all. It can just be a function in a different package; it can use events.get_current_event_loop() to reference the event loop. -- --Guido van Rossum (python.org/~guido)
On 18 January 2013 21:24, Guido van Rossum <guido@python.org> wrote:
Aargh. I'm confused again! (I did warn you about dumb questions, didn't I? :-)) The event loop implementation contains the code that does the OS-level poll for events to process. (In tulip, that is handled by the selector object, but that's not mentioned in the PEP so I assume it should be considered an implementation detail). So, the event loop has to define what types of (OS-level) objects can be registered. At the moment, event loops only handle sockets (via select/poll/etc) and even the raw add_reader methods are not for end user use. So a standalone create_corba_connection function can certainly get the event loop using get_current_event_loop(), but it has no means of asking the event loop to poll the CORBA connection it creates for new messages. Without direct access to the selector (or equivalent) it can't add the extra event source. (Unless that source is a pollable file descriptor and it's willing to play with the optional add_reader methods, but that's not a "new event source" then...) The same problem will likely occur if you try to integrate Windows GUI events (you check for a GUI message by calling a Windows API). I don't think this matters except in obscure cases (it's likely a huge case of YAGNI) but I genuinely don't understand how you can say that create_corba_connection() could be written as a standalone function, and yet that create_connection() has to be a method of the event loop. That's what I'm getting at when I keep saying that I see you treating sockets as "special". There's clearly something I'm missing in your thinking, and it keeps tripping me up. Paul.
On Fri, Jan 18, 2013 at 2:32 PM, Paul Moore <p.f.moore@gmail.com> wrote:
Well, *on UNIX* the event loop also handles other file descriptors, and there's nothing to actually *prevent* an end user using add_reader. It just may not work when their code is run on Windows, but then it probably won't run on Windows anyway. :-)
Right, unless it is in on the conspiracy between the event loop and the selector (IOW if it is effectively aware and/or part of the event loop implementation for the specific platform).
Let's say that you are thinking through the example much farther than I had intended... :-)
Let's assume that create_corba_connection() actually *can* be written using add_reader(), but only on UNIX. So the app is limited to UNIX, and in that context create_corba_connection() can be a function in another package. It's not so much that create_connection() *must* be a method on the event loop. It's just that I *want* it to be a method on the event loop so you will be able to write user code that is portable between UNIX and Windows. It will call create_connection(), which is a portable API with two platform-specific implementations; on Windows (when using IOCP) it will return an instance of, say, _IocpSocketTransport(), while on UNIX it returns a _UnixSocketTransport() instance. But we have no hope of making create_corba_connection() on Windows (in my example -- please just play along) and hence there is no need to make it a method of the event loop. -- --Guido van Rossum (python.org/~guido)
participants (14)
-
Andrew Bennetts -
Antoine Pitrou -
Ben Darnell -
Cameron Simpson -
Eli Bendersky -
Geert Jansen -
Glyph -
Greg Ewing -
Guido van Rossum -
Nick Coghlan -
Oleg Broytman -
Paul Moore -
Richard Oudkerk -
Shane Green