----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37336/#review107421 -----------------------------------------------------------
3rdparty/libprocess/include/process/subprocess.hpp (lines 53 - 75) <https://reviews.apache.org/r/37336/#comment166518> What's the motivation of storing this? Should the caller already have those information? 3rdparty/libprocess/include/process/subprocess.hpp (lines 114 - 117) <https://reviews.apache.org/r/37336/#comment166519> Ditto. I am not convinced that this is strictly necessary. 3rdparty/libprocess/include/process/subprocess.hpp (lines 119 - 130) <https://reviews.apache.org/r/37336/#comment166520> Why not just use a single `int status` field here. The users can use WEXITSTATUS ... to derive those information themself. This is also consistent with other places in the code place. 3rdparty/libprocess/include/process/subprocess.hpp (line 328) <https://reviews.apache.org/r/37336/#comment166521> I don't like the name 'execute'. When you create the Subprocess instance, the subprocss is already launched and exec'ed. This is rather waiting for the subprocess to terminate. 3rdparty/libprocess/include/process/subprocess.hpp (lines 336 - 343) <https://reviews.apache.org/r/37336/#comment166522> Why introduce this method? I think the caller should be responsible for killing the subprocess if needed. Also, os::killtree is in general not reliable and shouldn't be used in library code. 3rdparty/libprocess/src/subprocess.cpp (lines 185 - 190) <https://reviews.apache.org/r/37336/#comment166523> What if outData is called more than once? - Jie Yu On Nov. 10, 2015, 8:51 p.m., Marco Massenzio wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37336/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2015, 8:51 p.m.) > > > Review request for mesos, Joris Van Remoortere and Michael Park. > > > Bugs: MESOS-3035 > https://issues.apache.org/jira/browse/MESOS-3035 > > > Repository: mesos > > > Description > ------- > > The original API for `process::Subprocess` still left a lot of legwork > to do for the caller; we have now added an `execute()` method > that returns a `Future<Subprocess::Result>`. > > `Subprocess::Result`, also introduced with this patch, contains useful > information > about the command invocation (an `Invocation` struct); the exit code; > `stdout`; > and, optionally, `stderr` too. > > Once the Future completes, if successful, the caller will be able to retrieve > stdout/stderr; whether the command was successful; and whether it received a > signal > > > Diffs > ----- > > 3rdparty/libprocess/include/process/subprocess.hpp > f17816e813d5efce1d3bb1ff1e1111850eeda3ba > 3rdparty/libprocess/src/subprocess.cpp > efe0018d0414c4137fd833c153eb262232e712bc > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > ac600a551fb1a7782ff33cce204b7819497ef54a > > Diff: https://reviews.apache.org/r/37336/diff/ > > > Testing > ------- > > make check > > (also tested functionality with an anonymous module that exposes an > `/execute` endpoint and runs arbitrary commands, asynchronously, > on an Agent) > > > Thanks, > > Marco Massenzio > >
