stripe / subprocess

A port of Python's subprocess module to Ruby
MIT License
208 stars 17 forks source link

Add support for partial reads from Subprocess#communicate #36

Closed andrew-stripe closed 7 years ago

andrew-stripe commented 7 years ago

It can be useful to read partial output from a long running process before it completes (e.g. to provide feedback to the user about the progress). This adds the ability to pass a block to communicate that gets called with data read from stdout/stderr as its read instead of returning it.

r? @nelhage

nelhage commented 7 years ago

r? @nelhage-stripe

nelhage-stripe commented 7 years ago

I'm a little bit unhappy at the degree to which we're increasingly building out our own event loop / async IO framework, and I wonder if at some point we want to reimplement communicate on top of nio4r or something and bifurcate our usage into "simple things use open3, complex things use nio and a real event loop"

But at any given point in time it's always easier to add one-more-thing to #communicate, and this seems like an easy enough thing to add so

:+1:

(+1 to @russell-stripe's doc suggestions)

andrew-stripe commented 7 years ago

Updated the YARD bits in 907ce5b and fixed some other broken YARD syntax the server warned me about while I was at it. I also tweaked the logic slightly in 95f76e8 to only yield when we actually have content in stdout/stderr rather than any time we attempt to read from them (previously it could non-deterministically yield an empty string when the process exits).

I think nio4r would simplify the communicate nicely, though it's still pretty low level. I'm always a little wary of introducing dependencies that require C compilation in core utilities since that can fail in annoying ways.

I think Open3 is a plausible substitute but I'm a bit worried about how its use of threads will play with our penchant for forking.

nelhage-stripe commented 7 years ago

:+1: