> On March 3, 2016, 12:07 a.m., Avinash sridharan wrote: > > src/slave/flags.cpp, line 690 > > <https://reviews.apache.org/r/44200/diff/1/?file=1275006#file1275006line690> > > > > Maybe s/Directory path/Location ?
I just followed the existing convention, please see the help message of `Flags::launcher_dir` and `Flags::work_dir`, they all use the words "Directory path". > On March 3, 2016, 12:07 a.m., Avinash sridharan wrote: > > src/slave/flags.cpp, line 694 > > <https://reviews.apache.org/r/44200/diff/1/?file=1275006#file1275006line694> > > > > s/directory/location > > > > remove this line: > > This flag is used for\n" > > "the `network/cni` isolator.\n", > > "/tmp/mesos/cni/plugins" Did you mean removing the line: `This flag is used for the 'network/cni' isolator.`? I think this is the existing convention, please see the help message of `Flags::eth0_name`, `Flags::network_enable_snmp_statistics`, `Flags::container_disk_watch_interval`, etc. They all have the line like: `This flag is used for the 'xxx' isolator.`. > On March 3, 2016, 12:07 a.m., Avinash sridharan wrote: > > docs/configuration.md, line 1305 > > <https://reviews.apache.org/r/44200/diff/1/?file=1275002#file1275002line1305> > > > > Update the documentation based on the comments in `flags.cpp` Thanks for your comments in `flags.cpp` :-) I have replied your comments there, please let me know if you have further comments. > On March 3, 2016, 12:07 a.m., Avinash sridharan wrote: > > docs/endpoints/slave/state.json.md, line 91 > > <https://reviews.apache.org/r/44200/diff/1/?file=1275003#file1275003line91> > > > > Do we need to change some install scripts to create these paths? I > > think we should leave the paths as empty. Giving a default path implies > > that the path should exist during mesos installation. Leaving the path > > empty would force the operator to specify it while launching `network/cni` > > isolator. I think operator will be forced to specify it if the default path `/tmp/mesos/cni/plugins` does not exist because there is a check in `NetworkCniIsolatorProcess::create()`, please see https://reviews.apache.org/r/44269/ for details, if the default path does not exist, the agent will eventually exit with a meaningful error message to operator. - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44200/#review121661 ----------------------------------------------------------- On March 1, 2016, 3:04 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44200/ > ----------------------------------------------------------- > > (Updated March 1, 2016, 3:04 p.m.) > > > Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu. > > > Bugs: MESOS-4761 > https://issues.apache.org/jira/browse/MESOS-4761 > > > Repository: mesos > > > Description > ------- > > Add agent flags for specifying CNI plugin and config directories. > > > Diffs > ----- > > docs/configuration.md c4d094aac4acb95fd6648071413a3d5160dbf381 > docs/endpoints/slave/state.json.md a6403eea3426bdec81d170d0829ffe7174862e55 > docs/endpoints/slave/state.md bdd555e0231935c668dea1ba63582348db2b3629 > src/slave/flags.hpp c07932157ffe36ea36f1def4c7d92b79d2219c48 > src/slave/flags.cpp 7f139e99e8392fe95bb30070896ed9d95dfe5701 > src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e > > Diff: https://reviews.apache.org/r/44200/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Qian Zhang > >
