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<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 | mad...@cn.ibm.com | http://k82.me

<http://k82.me/>

Reply via email to