unix_pipes.PipeSendStream: add send_some method #783
Conversation
`send_some` does not block after its first write completes, and returns an indication of how much data it sent, so it can be used in cases where the amount of data sent before a cancellation must be known. This is needed to implement stdlib-compatible `subprocess.Popen.communicate()`.
Codecov Report
@@ Coverage Diff @@
## master #783 +/- ##
==========================================
+ Coverage 99.01% 99.01% +<.01%
==========================================
Files 96 96
Lines 11673 11713 +40
Branches 832 836 +4
==========================================
+ Hits 11558 11598 +40
Misses 94 94
Partials 21 21
Continue to review full report at Codecov.
|
|
I think we decided in chat that we weren't going to try to implement the stdlib |
|
Link to chat logs: https://gitter.im/python-trio/general?at=5bf5d64ced6bcf1ef83ccdb8 |
|
I just reviewed those chat logs again and I don’t think we actually decided anything - you pointed out that the stdlib behavior is unusual (which is true), I pointed out the usefulness of preserving at least the partial output across calls (which would require some funkiness even if it wouldn’t need |
|
Subprocesses are pretty complicated, and (Storing unbounded amounts of string data as state on the process object itself? Maybe it's a good idea, I dunno, but it's definitely super weird.) |
|
Fair enough. Making communicate() restartable seems like it would be a backwards-compatible change; I'm happy to leave that capability out of the initial version, which obviates the need for this diff. |
|
Like, is
Neither of these involves |
|
Hmm, that's a good point. I had been thinking "well of course run() is implemented in terms of communicate()" but there's no particular reason that needs to be the case. I think a non-restartable communicate() is not going to be too much of a burden to maintain and leaving it out would surprise people, but I'm open to other alternatives. |
|
Maybe! Let's move ahead with the stuff that we already know how to handle, and then revisit |
Adds `trio.subprocess.Process` and `trio.subprocess.run`, async wrappers for the stdlib subprocess module. No communicate() yet due to discussion on python-trio#783. Note that this did not wind up using the code in linux_waitpid, instead running waitid() [which supports not consuming the status of the process] in a thread, in order to interoperate better with subprocess.Popen code. Still TODO: [ ] Windows support (need pipe implementations and testing) [ ] documentation writeup, newsfragment [ ] consider whether this module layout is the one we want (other option would be to move _subprocess/unix_pipes.py to just _unix_pipes.py, _subprocess/highlevel.py to _subprocess.py, and get rid of _subprocess/linux_waitpid.py) [ ] expose other subprocess functions such as call, check_call, output (simple wrappers around run()) [ ] expose a basic communicate()?
Adds `trio.subprocess.Process` and `trio.subprocess.run`, async wrappers for the stdlib subprocess module. No communicate() yet due to discussion on python-trio#783. Note that this did not wind up using the code in linux_waitpid, instead running waitid() [which supports not consuming the status of the process] in a thread, in order to interoperate better with subprocess.Popen code. Still TODO: [ ] Windows support (need pipe implementations and testing) [ ] documentation writeup, newsfragment [ ] consider whether this module layout is the one we want (other option would be to move _subprocess/unix_pipes.py to just _unix_pipes.py, _subprocess/highlevel.py to _subprocess.py, and get rid of _subprocess/linux_waitpid.py) [ ] expose other subprocess functions such as call, check_call, output (simple wrappers around run()) [ ] expose a basic communicate()?
send_somedoes not block after its first write completes, and returnsan indication of how much data it sent, so it can be used in cases where
the amount of data sent before a cancellation must be known. This is
needed to implement stdlib-compatible
subprocess.Popen.communicate().