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



src/slave/slave.cpp
<https://reviews.apache.org/r/26622/#comment99004>

    I wonder if we instead should just copy the arguments we explicitly want? 
If we add another field to commandInfo that is not intended here it will break 
again.



src/slave/slave.cpp
<https://reviews.apache.org/r/26622/#comment99005>

    Ah I see, sorry I totally missed what's the point of this jira/rb.



src/tests/slave_tests.cpp
<https://reviews.apache.org/r/26622/#comment99001>

    delete the detector



src/tests/slave_tests.cpp
<https://reviews.apache.org/r/26622/#comment99002>

    bells and whistles doesn't seem to be a good description of what you're 
trying to do. Perhaps just say you're setting shell and arguments in the 
original TaskInfo and checking later those are not used to launch the command 
executor?


- Timothy Chen


On Oct. 18, 2014, 4:40 a.m., R.B. Boyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26622/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2014, 4:40 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-1873
>     https://issues.apache.org/jira/browse/MESOS-1873
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Basically if you use "shell=false" with a non-empty argument list and the 
> Command Executor it is completely broken.
> 
> When we clone the env vars to fork "mesos-executor" all of the original cmd 
> args are cloned as well (unintentionally) due to some protocol-buffer merge 
> shenanigans.
> 
> Don't pass task-related arguments to mesos-executor.
> 
> The description on the linked jira ticket goes into more detail.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp 0e342ed 
>   src/tests/slave_tests.cpp f585bdd 
> 
> Diff: https://reviews.apache.org/r/26622/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> added new test "SlaveTest.GetExecutorInfo" verifying the explicit desired 
> behavior.
> 
> 
> Thanks,
> 
> R.B. Boyer
> 
>

Reply via email to