> On May 25, 2016, 6:18 p.m., Jie Yu wrote: > > src/docker/executor.hpp, line 90 > > <https://reviews.apache.org/r/47205/diff/4/?file=1392872#file1392872line90> > > > > Can this be `Option<JSON::Object>`?
If I make this a `JSON::Object`, the invocation site (https://reviews.apache.org/r/47216/diff/2#1) will do one extra copy (see the next section): It would go: -> `map<string, string>` -> `map<string, JSON::Value>` -> `string` Instead of: -> `map<string, string>` -> `string` The parsing path remains unchanged, except there's one less `Try<JSON::Object>` in the executor's main. I prefer the current flag type. --- The conversion would look like: ``` map<string, string> task_environment = <something>; JSON::Object json; foreachpair (const string& key, const string& value, task_environment) { json.values[key] = value; } ``` - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47205/#review134882 ----------------------------------------------------------- On May 25, 2016, 7:02 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47205/ > ----------------------------------------------------------- > > (Updated May 25, 2016, 7:02 p.m.) > > > Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya. > > > Bugs: MESOS-5350 > https://issues.apache.org/jira/browse/MESOS-5350 > > > Repository: mesos > > > Description > ------- > > This flag opens up a way for hooks to specify environment variables for > docker tasks. Existing hooks can only affect the environment variables > of docker executors. > > > Diffs > ----- > > src/docker/executor.hpp 798ca3d4e261854a3b911d59929f2ca2afeb9ac8 > src/docker/executor.cpp 04f44044a80e1b355580ee3d1d8d301ac3baf565 > > Diff: https://reviews.apache.org/r/47205/diff/ > > > Testing > ------- > > See later reviews in chain. > > > Thanks, > > Joseph Wu > >
