----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48671/#review137438 -----------------------------------------------------------
Fix it, then Ship it! Looks good overall, just some small nits! src/slave/containerizer/mesos/containerizer.cpp (line 32) <https://reviews.apache.org/r/48671/#comment202563> You can probably safely remove the `#include` for `<utility>` and the `using std::pair` as they were only introduced for the recent ordering changes that are now being reverted. src/slave/containerizer/mesos/containerizer.cpp (line 347) <https://reviews.apache.org/r/48671/#comment202565> You probably don't want to print `+ isolation +` here as it will double print below. - Kevin Klues On June 14, 2016, 1:31 a.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48671/ > ----------------------------------------------------------- > > (Updated June 14, 2016, 1:31 a.m.) > > > Review request for mesos, Jie Yu and Kevin Klues. > > > Bugs: MESOS-5581 > https://issues.apache.org/jira/browse/MESOS-5581 > > > Repository: mesos > > > Description > ------- > > In 6fb9c024, the ordering of isolators was changed from being > specified by the operator to being specified by the code. It > turns out that the operator ordering was intended to ensure > that isolators can check their ordering requirements. If an > isolator detects that its ordering requirements are not > met, it can inform the operator to adjust the --isolation flag > via a failure message. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.cpp > e5a339d965aeef2e3289c0ec3d9fb6eb281a567c > src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp > d7557a0c338e8c0e51461b2326600c03f89c2e8b > > Diff: https://reviews.apache.org/r/48671/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Mahler > >