> On July 11, 2016, 8:23 p.m., Jie Yu wrote: > > src/slave/main.cpp, lines 175-179 > > <https://reviews.apache.org/r/46298/diff/2/?file=1349891#file1349891line175> > > > > Can we do that check in `add` function. `add` function supports an > > optional validate lambda to be passed in. > > Klaus Ma wrote: > @jie, thanks for the comments. Try to use validated lamba today, but > there some issues: > > 1. The `work_dir` is required without default value; if using validate > lamba, a "default" value is necessary: `add(T1* t1, name, alias, > defaultValue, lamba)` > 2. The default value is pass by const reference currently, we can not > pass some empty value to it, e.g. nullptr or 0 > > For the default value, I'm thinking to replace const reference with > `Option`; any suggestion?
Oh, I don't realize that we don't have validation support for required field without default value. Can we introduce another `add` variant in flags that support the above case? Is that difficult? - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46298/#review141756 ----------------------------------------------------------- On April 19, 2016, 7:31 a.m., Klaus Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46298/ > ----------------------------------------------------------- > > (Updated April 19, 2016, 7:31 a.m.) > > > Review request for mesos, Alexander Rukletsov and Jie Yu. > > > Bugs: MESOS-5123 > https://issues.apache.org/jira/browse/MESOS-5123 > > > Repository: mesos > > > Description > ------- > > Rejected relative path agent work_dir. > > > Diffs > ----- > > src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 > src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 > > Diff: https://reviews.apache.org/r/46298/diff/ > > > Testing > ------- > > 1. make && make check > 2. e2e test: > > ``` > $ ./src/mesos-slave --work_dir=aa --master=aa > The required option `--work_dir` must be absolute path. > ``` > > > Thanks, > > Klaus Ma > >
