I'm shepherding https://issues.apache.org/jira/browse/MESOS-3335, and I'm hoping to we don't run into too much conflicts here in terms of rebasing and such.
Do you guys have anything concrete at this point and a shepherd who will help you guys with this stuff? If not, I'm up to shepherding it, but after MESOS-3335. Thanks, MPark On 14 July 2016 at 07:39, Klaus Ma <[email protected]> wrote: > Agree with you clean up `FlagBase::add` :). > > alexR/bbannier/neil is also working on enhancement to the `FlagBase` > (MESOS-3335), I think > we can work together to make `FlagBase::add` more clear. > > Thanks > Klaus > > On Jul 14, 2016, at 02:08, Greg Mann <[email protected]<mailto: > [email protected]>> wrote: > > Thanks for bringing this up, Klaus - in this case, I think that extra > argument appears simply to match the desired function overload. Over time, > the overloads for `FlagsBase::add` have multiplied considerably; It looks > like we have about 20 now :-) I think it would be really nice to clean > these up somehow. I didn't see a JIRA issue for this improvement so I > created one: MESOS-5841 <https://issues.apache.org/jira/browse/MESOS-5841> > > One option would be to get rid of all the overloads except for > `FlagsBase::add(const Flag& flag)`, add a couple helper functions for > modifying `Flag` objects, and construct flag objects in the 'flags.cpp' > files: > > Flag flag; > flag.name = "work_dir"; > flag.help = help_string; > flag.set_storage(&Flags::work_dir); // New helper > flag.set_validation(lambda_function); // New helper > add(flag); > > I think this would make the 'flags.cpp' files more readable, and it would > clean up `FlagsBase` by getting rid of all those overloads. > > Cheers, > Greg > > > On Tue, Jul 12, 2016 at 11:14 PM, Klaus Ma <[email protected]<mailto: > [email protected]>> wrote: > > Hi team, > > > When I updating the patch for MESOS-5123< > https://issues.apache.org/jira/browse/MESOS-5123>, I found the validate > lamba function for in `Flags::add` for required parameters is different > with optional parameters. Does any know why? The coding style is > inconsistent, it took times to find the suitable function :). > > > Flags::add for optional parameters: > > ``` > > add(&Flags::executor_environment_variables, > > "executor_environment_variables", > > "JSON object representing the environment variables that should be\n" > > "passed to the executor, and thus subsequently task(s). By default > this\n" > > "flag is none. Users have to define executor environment > explicitly.\n" > > "Example:\n" > > "{\n" > > " \"PATH\": \"/bin:/usr/bin\",\n" > > " \"LD_LIBRARY_PATH\": \"/usr/local/lib\"\n" > > "}", > > [](const Option<JSON::Object>& object) -> Option<Error> { > > if (object.isSome()) { > > foreachvalue (const JSON::Value& value, object.get().values) { > > if (!value.is<http://value.is><JSON::String>()) { > > return Error("`executor_environment_variables` must " > > "only contain string values"); > > } > > } > > } > > return None(); > > }); > > ``` > > > Flags::add for required parameters: > > > ``` > > add(&Flags::work_dir, > > "work_dir", > > None(), // <============= Additional parameters to Flags::add > > "Absolute directory path of the agent work directory. This is > where\n" > > "executor sandboxes will be placed, as well as the agent's > checkpointed\n" > > "state in case of failover. Note that locations like `/tmp` which > are\n" > > "cleaned automatically are not suitable for the work directory > when\n" > > "running in production, since long-running agents could lose data > when\n" > > "cleanup occurs; if launching docker tasks, the path must not > include\n" > > "any disallowed symbols for docker volumes.\n" > > "(Example: `/var/lib/mesos/agent`)", > > static_cast<const string*>(0), > > [](const string& workDir) -> Option<Error> { > > if (!strings::startsWith(workDir, "/")) { > > return Error( > > "The required option `--work_dir` must be absolute path."); > > } > > return None(); > > }); > > ``` > > > ---- > > Da (Klaus), Ma (??), PMP®| Software Architect > Platform DCOS Development & Support, STG, IBM GCG > +86-10-8245 4084 | [email protected]<mailto:[email protected]> | > http://k82.me > > <http://k82.me/> > >
