----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20141/#review40323 -----------------------------------------------------------
Please comment the src/dest of each protobuf message, and don't make so many fields required. include/mesos/containerizer.proto <https://reviews.apache.org/r/20141/#comment73320> Sent from Mesos slave to external containerizer (e.g. Deimos)? Let's explicitly document the to/from and direction of these messages. include/mesos/containerizer.proto <https://reviews.apache.org/r/20141/#comment73308> Are all of these fields always going to be required by every external containerizer until the end of time? As the protobuf docs say, "Required is forever." It's a lot safer to use optional just in case anybody ever wants to change the type, deprecate the field, or make it repeated. Figure out what the 1 or 2 absolutely-required, always-singular fields are (ContainerID, FrameworkID?) and make everything else optional. include/mesos/containerizer.proto <https://reviews.apache.org/r/20141/#comment73321> Is this the external containerizer updating the slave/executor, or vice versa? include/mesos/containerizer.proto <https://reviews.apache.org/r/20141/#comment73316> optional? In case we just want to update other aspects of the container (user, frameworkId, uris, etc.)? include/mesos/containerizer.proto <https://reviews.apache.org/r/20141/#comment73322> Termination message comes from the (external) containerizer to the slave, right? include/mesos/containerizer.proto <https://reviews.apache.org/r/20141/#comment73314> optional? - Adam B On April 10, 2014, 7:03 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20141/ > ----------------------------------------------------------- > > (Updated April 10, 2014, 7:03 p.m.) > > > Review request for mesos, Benjamin Hindman and Niklas Nielsen. > > > Repository: mesos-git > > > Description > ------- > > Adds containerizer.proto containing protobuf message definitions for > "Launch", "Update" and "Termination". All of those are used in the upcoming > External Containerizer. > These protos are defined within their own C++ namespace: "containerizer". > > Replaced slave::Containerizer::Termination with the > containerizer::Termination protobuf, gaining a serializable Termination > message. > > The protoc results are added towards the Python Egg (containerizer_pb2.py) > and the Java package (ContainerizerProtos.java). > > > Diffs > ----- > > include/mesos/containerizer.proto PRE-CREATION > src/Makefile.am 95f133d > src/slave/containerizer/containerizer.hpp d9ae326 > src/slave/containerizer/mesos_containerizer.hpp ee1fd30 > src/slave/containerizer/mesos_containerizer.cpp 1ce41d7 > src/slave/slave.hpp 08f6005 > src/slave/slave.cpp cddb241 > src/tests/containerizer.hpp a9f1531 > src/tests/containerizer.cpp bfb9341 > > Diff: https://reviews.apache.org/r/20141/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
