> On Nov. 7, 2015, 1:06 a.m., Michael Park wrote: > > 3rdparty/libprocess/include/process/subprocess.hpp, line 340 > > <https://reviews.apache.org/r/37336/diff/10/?file=1117802#file1117802line340> > > > > I think this violates our use of `auto` rule. Could you please spell > > out the type? > > Marco Massenzio wrote: > I would disagree here... the type is `Try<std::list<os::ProcessTree>>` > and adding that, I don't think makes the code any more readable (on the > contrary). > In fact, we don't really make use of any of that - all we care is that no > error is returned: it could very well be a `Try<Pizza>` for all we care :) > > This seems to me a poster child of the use-case for `auto`... anyways, no > biggie, I've added the type.
Yes, you are correct that the only part we care about is the `Try`. In the future, we'll be able to say `Try<auto> result = ...;` exactly in these cases. As is, however we either have to go with `auto` or `Try<std::list<os::ProcessTree>>` and neither of them are ideal. In the context of Mesos however, we tend to err on the side of overcommunicating the type. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37336/#review105544 ----------------------------------------------------------- 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 > >
