----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48527/#review138128 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/cni/cni.hpp (line 92) <https://reviews.apache.org/r/48527/#comment203344> Why 'taskId' here? If it's a command task, that's clear what taskId is. But what if this is a custom executor with multiple tasks, what is this id for? src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 968 - 971) <https://reviews.apache.org/r/48527/#comment203347> I would prefer the following for the `args` json: ``` { ... "args" : { "mesos" : { "network_info" : { ... // JSON rep. of NetworkInfo. } } } } ``` src/slave/containerizer/mesos/isolators/network/cni/paths.cpp (lines 126 - 135) <https://reviews.apache.org/r/48527/#comment203348> I would prefer not introducing a new checkpointed file which we will deprecate soon. In the future, we need to checkpoint the network config json anyway because technically, when we call `plugin DEL`, we need to provide an identical network config as we provide to `plugin ADD`. I think the previous way of using a PIPE for stdin sounds fine to me. - Jie Yu On June 15, 2016, 10:23 p.m., Dan Osborne wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48527/ > ----------------------------------------------------------- > > (Updated June 15, 2016, 10:23 p.m.) > > > Review request for mesos and Jie Yu. > > > Repository: mesos > > > Description > ------- > > Review: https://reviews.apache.org/r/48527 > > Fixes. > > Review: https://reviews.apache.org/r/48754 > > Code review fixes. > > > Checkpointed mutated networkinfo. > > > Added error message if args is specified. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp > 09890cedf2e7a1846bd1cb250e117be1680a1b80 > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 106ff35320f37b2e75ed60381de1406459e6d515 > src/slave/containerizer/mesos/isolators/network/cni/paths.hpp > 8af50c11b00240ac1d24605b119acbd96aaa50be > src/slave/containerizer/mesos/isolators/network/cni/paths.cpp > b3fb0a32e3e74ca859ca58085efed5a87e4e626b > > Diff: https://reviews.apache.org/r/48527/diff/ > > > Testing > ------- > > > Thanks, > > Dan Osborne > >
