> On Nov. 20, 2015, 10:12 p.m., Jie Yu wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, line 328 > > <https://reviews.apache.org/r/37336/diff/11/?file=1122402#file1122402line328> > > > > 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. > > Marco Massenzio wrote: > This method most definitely does **not** wait. > > This is how one can use it as a caller (code simplified): > ``` > auto s = process::subprocess(commandInfo.command(), args); > > if (s.isError()) { > LOG(ERROR) << "Could not spawn subprocess: " << s.error(); > return http::ServiceUnavailable(s.error()); > } > > store(s.get().pid()); // <-- needed to reconcile with GETs > > Future<CommandResult> result_ = s->execute(); > result_.then([](const Future<CommandResult> &future) { > if (future.isFailed()) { > // mark the command as failed > return Nothing(); > } > auto result = future.get(); > // update status of job - use pid(); something equivalent to: > LOG(INFO) << "Result of '" << result.invocation.command << "'was: > " > << result.stdout(); > return Nothing(); > }).after(Seconds(30), [s](const Future<Nothing> &future) { > // update status of job to timed out; use `invocation` and > `stdout`. > s.get().cleanup(); > return Nothing(); > }); > > http::Response response = http::OK("{\"result\": \"OK\", \"pid\": \"" + > stringify(s.get().pid()) + "\"}"); > response.headers["Content-Type"] = "application/json"; > return response; > ``` > > Jie Yu wrote: > From the code above, can you just caputure commandInfo.command() in the > lambda and print it? > > ``` > string command = commandInfo.command(); > > result_.then([command](...) { > ... > LOG(INFO) << command << "..."; > }); > ```
ALso, `auto s = process::subprocess(commandInfo.command(), args);` this line fork and exec the subprocess. So having another `s->execute()` sounds very confusing to me. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37336/#review107421 ----------------------------------------------------------- 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 > >