----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28975/#review65900 -----------------------------------------------------------
Looking good. Just the protobuf variables to snake_case, since that's how protobufs auto-generate their accessors. include/mesos/fetcher/fetcher.proto <https://reviews.apache.org/r/28975/#comment109176> Protobufs use snake case, so command_info, work_directory, frameworks_home, and hadoop_home. src/launcher/fetcher.cpp <https://reviews.apache.org/r/28975/#comment109171> Might this want to be `using namespace mesos::fetcher` in the future, if there are more messages added there? Probably doesn't matter yet. src/launcher/fetcher.cpp <https://reviews.apache.org/r/28975/#comment109175> How can fetcherInfo not have a commandinfo if it's required? I would think it would have errored in the parse function. src/launcher/fetcher.cpp <https://reviews.apache.org/r/28975/#comment109178> Ditto on another required field, but maybe check that it's not empty string? src/launcher/fetcher.cpp <https://reviews.apache.org/r/28975/#comment109181> const string& ? src/slave/containerizer/fetcher.hpp <https://reviews.apache.org/r/28975/#comment109182> Are we going to do this sometime soon too? - Adam B On Dec. 11, 2014, 4:48 p.m., Bernd Mathiske wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28975/ > ----------------------------------------------------------- > > (Updated Dec. 11, 2014, 4:48 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-2173 > https://issues.apache.org/jira/browse/MESOS-2173 > > > Repository: mesos-git > > > Description > ------- > > Changed env var ensemble into one env var containing JSON, parsing this to > protobuf for use. Adapted fetcher tests. > > > Diffs > ----- > > include/mesos/fetcher/fetcher.hpp PRE-CREATION > include/mesos/fetcher/fetcher.proto PRE-CREATION > src/Makefile.am 86161fe7a8bdd86958d24adb74d434cd92d7dfb8 > src/launcher/fetcher.cpp 2f54d9e061d6b56a9b9cd857e5a908fee2847e68 > src/slave/containerizer/fetcher.hpp > 12b81b1065b6b0a0368ebd44326c75c1d4e4ba79 > src/slave/containerizer/fetcher.cpp > d702a9ccc15b21ba26fb9356aa69da15dfd5973e > src/tests/fetcher_tests.cpp 9e48392d31c886bad60abc8999fe293f55f589b0 > > Diff: https://reviews.apache.org/r/28975/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Bernd Mathiske > >
