> On Oct. 23, 2014, 9:11 p.m., Timothy Chen wrote: > > src/tests/slave_tests.cpp, line 420 > > <https://reviews.apache.org/r/26622/diff/2/?file=719278#file719278line420> > > > > 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? > > R.B. Boyer wrote: > I totally won't mind if you rewrite the commentary and unit tests here. > I've barely read any of the mesos source (only enough to identify this bug) > so I was largely submitting the patch to accelerate the actual fix from > someone who understands the code and the right way to fix this.
We usually let the submitter finish all the code changes, and commit it with your name/email so you get the credit of doing this. If you want I can totally take over and rewrite your fix. - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review58102 ----------------------------------------------------------- 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 > >
