> On July 15, 2015, 12:36 a.m., Artem Harutyunyan wrote:
> > src/slave/slave.cpp, line 4755
> > <https://reviews.apache.org/r/36389/diff/4/?file=1011886#file1011886line4755>
> >
> >     Shouldn't there be an equivalent of an assert here if we never expect this to happen? Something like this: 
> >     
> >     if (response.isReady()) {
> >          ASSERT
> >     }
> >     
> >     return http::BadRequest ...
> >     
> >     Unless, there is possibility of a race where the result becomes ready right at the 15th second (in a separate thread) and by the time this lambda is executed the response becomes actually (and legitimately) ready.

I sat down with Joris to verify whether or not there was a possible race condition resulting in a ready Future inside the `.after` lambda, and turned out that there is not. It's a great idea to have an explicit check for isReady() to be false, but the assert (i.e. CHECK) still does looks like a better option.


- Artem


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36389/#review91723
-----------------------------------------------------------


On July 14, 2015, 4:20 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36389/
> -----------------------------------------------------------
> 
> (Updated July 14, 2015, 4:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Cody Maloney.
> 
> 
> Bugs: MESOS-2830
>     https://issues.apache.org/jira/browse/MESOS-2830
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Jira: MESOS-2830
> 
> Under certain (maintenance) circumnstance, it may be necessary
> (or desirable) to execute arbitrary operator's commands on the
> slave (the entire fleet or a subset thereof) bypassing the Mesos
> Task execution mechanism; this may typically be necessary for
> maintenance and/or emergency actions.
> 
> This patch adds an HTTP endpoint (/execute) which accepts a
> JSON-encoded CommandInfo structure and executes the given
> command (with optional arguments).
> 
> The output of the command (along with potentially any stderr
> messages) is returned JSON-encoded in the Response.
> 
> For more details, see the design doc at:
> https://goo.gl/4npTMU
> 
> 
> Diffs
> -----
> 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/main.cpp 8008430e98773d8be9ba5ac6385cffb2e351932a 
>   src/slave/slave.hpp dec4ca8323e151a6d0f9139214ff0ef6e3e3375a 
>   src/slave/slave.cpp 2119b5176aa7cfb7b0b551d4d4f65ee12818b9e4 
> 
> Diff: https://reviews.apache.org/r/36389/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> lots of manual testing (using Postman, REST client)
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>