----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review57997 -----------------------------------------------------------
src/slave/slave.cpp <https://reviews.apache.org/r/26622/#comment98911> Thanks for the fix! I wonder if we should just fix the command executor instead? I usually don't like mingling with user API input as it always confuses whoever using this, and leads to people start asking mailing lists and reading code because their arguemnts are changed. Shell=false was added for a reason and I think we shouldn't just change it for them, did you see why we can't just support this variation in 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 > >
