----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71034/#review216482 -----------------------------------------------------------
Fix it, then Ship it! src/docker/executor.cpp Line 401 (original), 404 (patched) <https://reviews.apache.org/r/71034/#comment303715> Should we print the override value here if it is provided? src/docker/executor.cpp Lines 948 (patched) <https://reviews.apache.org/r/71034/#comment303713> Unnecessary `move`? src/docker/executor.cpp Lines 992 (patched) <https://reviews.apache.org/r/71034/#comment303714> Same as above, unnecessary `std::move()`. src/exec/exec.cpp Lines 374 (patched) <https://reviews.apache.org/r/71034/#comment303716> I believe the "standard" way to write this would be ``` auto* dockerExecutor = dynamic_cast<docker::DockerExecutor*>(executor); if (dockerExecutor) { // `executor` was a `DockerExecutor` } else { // `executor` was some other `Executor` } ``` This way, classes derived from `DockerExecutor` will also get the correct overload. - Benno Evers On July 8, 2019, 6:25 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71034/ > ----------------------------------------------------------- > > (Updated July 8, 2019, 6:25 p.m.) > > > Review request for mesos, Benno Evers, Benjamin Mahler, and Joseph Wu. > > > Bugs: MESOS-9853 > https://issues.apache.org/jira/browse/MESOS-9853 > > > Repository: mesos > > > Description > ------- > > This adds a new `killTask()` overload to the Docker executor > and updates the Mesos executor driver to call into that > overload when the loaded executor is the Docker executor. > > This allows the executor driver to pass the kill policy > override, when present, into the Docker executor. > > > Diffs > ----- > > src/docker/executor.hpp f21e84c71f646e84404c65fc2ded64bcaff482ef > src/docker/executor.cpp f638e4b65155bcca1be36424b7061ea26a3d6ca3 > src/exec/exec.cpp c0fa3b61667da96bc4395bae9956c54446268122 > > > Diff: https://reviews.apache.org/r/71034/diff/1/ > > > Testing > ------- > > Details at the end of this chain. > > > Thanks, > > Greg Mann > >
