----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58903/#review174637 -----------------------------------------------------------
Fix it, then Ship it! src/slave/containerizer/mesos/containerizer.cpp Lines 984-986 (original), 931-933 (patched) <https://reviews.apache.org/r/58903/#comment247834> In fact, I would call the parameter `_containerConfig` here: ``` ContainerConfig containerConfig = _containerConfig; ``` And moving forward, consistently use `containerConfig` in this function. src/slave/containerizer/mesos/containerizer.cpp Line 1066 (original), 1037 (patched) <https://reviews.apache.org/r/58903/#comment247832> It's weird that you still `containerConfig` not `containerConfig_` here. See my comments above. - Jie Yu On May 2, 2017, 2:04 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58903/ > ----------------------------------------------------------- > > (Updated May 2, 2017, 2:04 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7449 > https://issues.apache.org/jira/browse/MESOS-7449 > > > Repository: mesos > > > Description > ------- > > This simplifies the container launch path by removing combining > the nested and non-nested container code paths into one. > > The Mesos containerizer was originally translating the two > `containerizer->launch` entrypoints into a common method (also > called `launch`). The previous commits moved this translation > logic into the caller (i.e. the Agent). > > The end result has some slight changes: > * It is now possible for the Agent to specify some more combinations > of `ContainerConfig`. For example, specifying a TaskInfo with > a DEBUG-class container. Or a nested container with Resources. > We may need to add extra validation around this. > * The `bool checkpoint` argument was replaced with a `Option<string>` > which optionally contains an absolute path. This allows us to > remove the `SlaveID` field. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.hpp > 29a99f33e646593127b9dc126f398f7bca88e21d > src/slave/containerizer/mesos/containerizer.cpp > b58baed64480e22f640a4852537f85922ed382ae > > > Diff: https://reviews.apache.org/r/58903/diff/1/ > > > Testing > ------- > > See last patch in chain. > > > Thanks, > > Joseph Wu > >
