----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64575/#review193893 -----------------------------------------------------------
Fix it, then Ship it! src/slave/containerizer/mesos/containerizer.hpp Lines 360 (patched) <https://reviews.apache.org/r/64575/#comment272562> This is not quite true. Even if `config` is not optional anymore, `ContainerConfig.container_class` might not be set and default to `0`. Please kill this line. src/slave/containerizer/mesos/containerizer.cpp Lines 2877-2884 (patched) <https://reviews.apache.org/r/64575/#comment272561> Since this function can be used not only in the launch path where we have the container, you should check `config` is not `None`. How about something like this then? ``` return (config.isSome() && config->has_container_class()) ? config->container_class() : ContainerClass::DEFAULT; `` - Alexander Rukletsov On Dec. 13, 2017, 7:47 p.m., Armand Grillet wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64575/ > ----------------------------------------------------------- > > (Updated Dec. 13, 2017, 7:47 p.m.) > > > Review request for mesos and Alexander Rukletsov. > > > Bugs: MESOS-7361 > https://issues.apache.org/jira/browse/MESOS-7361 > > > Repository: mesos > > > Description > ------- > > This function always returns a ContainerClass, `DEFAULT` being the > default value returned. Also simplifies the conditional statements > in mesos/containerizer.cpp to use the function added. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.hpp > 965e183bb5c54f31d90e910edd35313ab380cea9 > src/slave/containerizer/mesos/containerizer.cpp > 7ab0b07f689f872573ca458ae47cd6426ebc0365 > > > Diff: https://reviews.apache.org/r/64575/diff/2/ > > > Testing > ------- > > ``` > GTEST_FILTER="" nice make check -j16 V=0 > ``` > > > Thanks, > > Armand Grillet > >
