----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48428/#review136726 -----------------------------------------------------------
Fix it, then Ship it! LGTM src/docker/docker.cpp (line 699) <https://reviews.apache.org/r/48428/#comment201947> Is there any reason why this `.onDiscard` isn't chained onto the `return s->status()` below instead of the `s->status()` above? src/slave/containerizer/docker.cpp (line 1239) <https://reviews.apache.org/r/48428/#comment201976> Typo: s/propgage/propagate/ src/slave/containerizer/docker.cpp (lines 1246 - 1248) <https://reviews.apache.org/r/48428/#comment201982> Since the `run`'s failure is now propagated manually below, is it still necessary to associate the `inspect` future within a callback? i.e. rather than ``` promise->associate(inspect); ``` - Joseph Wu On June 8, 2016, 8:57 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48428/ > ----------------------------------------------------------- > > (Updated June 8, 2016, 8:57 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-4279 > https://issues.apache.org/jira/browse/MESOS-4279 > > > Repository: mesos > > > Description > ------- > > Previously, Docker::run would return a Future<Nothing> which > requires the caller to perform an inspect to obtain the exit > status of the container. This is difficult to get right, and > so this patch leverages the fact that 'docker run' already > returns the exit status from the container termination to > simplify the calling code. Namely, in the docker executor we > need to use the exit status to determine how to send a > terminal status update (see the command executor code). > > > Diffs > ----- > > src/docker/docker.hpp d9ffc18496718701fac8182506a8c36e21e9c319 > src/docker/docker.cpp ffbd7ef56d02dfe550a60851251e6645d2ca5925 > src/docker/executor.cpp 1b3a7795b1db83394d4b884c1041c341f88a7df1 > src/slave/containerizer/docker.cpp 63efbe6f45958d44d60fe4a7fea816f5fb0457b2 > src/tests/containerizer/docker_containerizer_tests.cpp > 5591a6784afd10b4c7535f90c2e6745fa74c318b > src/tests/containerizer/docker_tests.cpp > 7ef52ade0d3389f9e24e3c5c7dda4f8809b9d83f > src/tests/mesos.hpp 6f2888023c957f0af4f7374c98e406816a8423e3 > > Diff: https://reviews.apache.org/r/48428/diff/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Benjamin Mahler > >
