----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67354/#review204681 -----------------------------------------------------------
include/mesos/executor.hpp Lines 220-223 (patched) <https://reviews.apache.org/r/67354/#comment287291> Maybe add this comment to the previous constructor saying that the other one is preferable? I think that way it will be easier for users to figure out they use a not favoured c-tor : ) src/exec/exec.cpp Line 646 (original), 654 (patched) <https://reviews.apache.org/r/67354/#comment287292> I doubt this does what you want it to do. At the end, `FlagsBase` calls `extract()` on the prefix, which in turn calls `os::environment()`. Please either mention here that env is loaded under the hood (as Ilya suggested before), or fix the flags to not load the environment at all. Maybe doing the latter should be part of a bigger change where we make `FlagsBase` instances never load from the environment (but provide a static function to do so). I will be happy to shepherd this work and chain it immediately after this change so it does not fall through the cracks. - Alexander Rukletsov On May 29, 2018, 2:20 p.m., Andrei Budnik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67354/ > ----------------------------------------------------------- > > (Updated May 29, 2018, 2:20 p.m.) > > > Review request for mesos, Alexander Rukletsov, Benjamin Bannier, haosdent > huang, Ilya Pronin, and James Peach. > > > Bugs: MESOS-3475 > https://issues.apache.org/jira/browse/MESOS-3475 > > > Repository: mesos > > > Description > ------- > > This patch adds overloaded constructor for `MesosExecutorDriver` that > accepts `environment` parameter and stores it in the class variable. > This new constructor is needed to get rid of `os::getenv()` calls, > so that `MesosExecutorDriver` can be used in tests that require > thread safety. > > > Diffs > ----- > > include/mesos/executor.hpp d14c0369f6731100d27092142b56f108f8881003 > src/exec/exec.cpp 65a671d7ce83a51087d290ba039d18deba6313c2 > > > Diff: https://reviews.apache.org/r/67354/diff/2/ > > > Testing > ------- > > internal CI > > > Thanks, > > Andrei Budnik > >
