----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52879/#review152940 -----------------------------------------------------------
src/launcher/default_executor.cpp (lines 1015 - 1042) <https://reviews.apache.org/r/52879/#comment222132> Thanks for doing that! This definitely simplies the parsing of those env variables. I think there are two types of flags (or ENV) that'll be passed to default executor (and this also applies to the old command executor): 1) executor environment variables that are *COMMON* to all executors, including custom executors. For instance, MESOS_EXECUTOR_ID, MESOS_FRAMEWORK_ID, MESOS_SANDBOX. 2) flags or env variables that only apply to default executor (or old command executor). For instance, `launcher_dir`, `MESOS_HTTP_COMMAND_EXECUTOR`. In retrospect, we probably should do the following for command executor: ``` COMMAND_EXECUTOR_HTTP_API COMMAND_EXECUTOR_LAUNCHER_DIR ``` and in command executor, we probably need two sets of flags: ``` MesosFlags mesosFlags; // MESOS_* CommandExecutorFlags commandExecutorFlags; // COMMAND_EXECUTOR_* ``` cc @vinodkone @anand src/launcher/default_executor.cpp (line 1028) <https://reviews.apache.org/r/52879/#comment222135> What's this? Is this MESOS_SANDBOX? src/launcher/default_executor.cpp (line 1038) <https://reviews.apache.org/r/52879/#comment222133> s/string/ExecutorID/ you may need to add a parse function for ExecutorID src/launcher/default_executor.cpp (line 1039) <https://reviews.apache.org/r/52879/#comment222134> Ditto. src/launcher/default_executor.cpp (lines 1056 - 1059) <https://reviews.apache.org/r/52879/#comment222136> I thought we should start to use `LIBPROCESS_SSL_xxx` src/launcher/executor.cpp (lines 883 - 885) <https://reviews.apache.org/r/52879/#comment222137> Ditto. Let's separate the two sets of flags. - Jie Yu On Oct. 14, 2016, 5:17 p.m., Gastón Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52879/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2016, 5:17 p.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, Jie Yu, and Jiang Yan Xu. > > > Repository: mesos > > > Description > ------- > > Updated the command and the default executor so that they use only > `stout::flags` to load config options. > > They used to use a mix of `stout::flags` and `os::getenv`. > > > Diffs > ----- > > src/launcher/default_executor.cpp af4a97f7de5f2157aa65fdab742455b0683c40a4 > src/launcher/executor.cpp 3e95d6029bea0ce6e0dfb39c24b795fe98d90d13 > > Diff: https://reviews.apache.org/r/52879/diff/ > > > Testing > ------- > > `make check` > > > Thanks, > > Gastón Kleiman > >
