----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45067/#review124377 -----------------------------------------------------------
Fix it, then Ship it! Ship It! src/examples/long_lived_framework.cpp (line 135) <https://reviews.apache.org/r/45067/#comment186892> I think use ``` if (flags.master.isNone()) { EXIT(EXIT_FAILURE) << flags.usage("Missing required option --master"); } ``` to keep consistent with current codebase would be better src/examples/long_lived_framework.cpp (line 144) <https://reviews.apache.org/r/45067/#comment186893> Should use `build_dir`? src/examples/long_lived_framework.cpp (line 145) <https://reviews.apache.org/r/45067/#comment186894> There is a duplicated space between `Mesos. If`. I think should be `Mesos. If`. src/examples/long_lived_framework.cpp (line 186) <https://reviews.apache.org/r/45067/#comment186895> Seems we never use `flags::mesos_build_dir`. By the way, if we use `flags.load(PREFIX)`, it would load the flag by environment variable as well so that we no need to check it here again. src/examples/long_lived_framework.cpp (line 210) <https://reviews.apache.org/r/45067/#comment186896> I suggest pull down ``` // Find this executable's directory to locate executor. string executor_command; Option<string> value = os::getenv("MESOS_BUILD_DIR"); if (value.isSome()) { executor_command = path::join(value.get(), "src", "long-lived-executor"); } else { executor_command = path::join( os::realpath(Path(argv[0]).dirname()).get(), "long-lived-executor"); } ``` and ``` executor.mutable_command()->set_value(executor_command); ``` here, because they are a logic block to set `executor.mutable_command()` - haosdent huang On March 19, 2016, 1:06 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45067/ > ----------------------------------------------------------- > > (Updated March 19, 2016, 1:06 a.m.) > > > Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Kevin Klues. > > > Repository: mesos > > > Description > ------- > > This gives the example `long-lived-framework` enough options to run outside > of the build environment. > > This also updates the style of the framework code and allows the > `ExecutorInfo` to be run with some cgroups isolators. > > > Diffs > ----- > > src/examples/long_lived_framework.cpp > 0000289a0b9dd3d1ce30f20dd9bb381126bff30c > > Diff: https://reviews.apache.org/r/45067/diff/ > > > Testing > ------- > > make check > > Ran this on the master node on a Mesos cluster: > ``` > ./long-lived-framework --master=zk://localhost:2181/mesos > --executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"; > --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && > ./long-lived-executor" > ``` > > > Thanks, > > Joseph Wu > >
