> On July 19, 2015, 8:09 p.m., Benjamin Hindman wrote: > > src/master/main.cpp, line 139 > > <https://reviews.apache.org/r/36425/diff/4/?file=1010039#file1010039line139> > > > > We've tried to name our flags the same as how you'd see them on the > > command line, so s/ipDiscoveryScript/ip_discovery_script/ here please. > > > > Also, what about s/script/command/ here? Since we might not be > > executing a script here and it's more inline with the fact that we'll be > > executing this via the shell (i.e., it's a shell command, as opposed to say > > doing s/script/executable/ which might imply we're going to fork/exec).
I'd actually asked about this to Nik - so long as we're clear that we're breaking the style guide here, I guess it's ok. There's also no good reason for it (apart from "tradition"? or "consistency"?) as we could have the default "snake_to_camelCase" convention for flags. http://despair.com/collections/demotivators/products/consistency > On July 19, 2015, 8:09 p.m., Benjamin Hindman wrote: > > src/master/main.cpp, lines 181-209 > > <https://reviews.apache.org/r/36425/diff/4/?file=1010039#file1010039line181> > > > > Apologies for not giving you this feedback sooner, but we already have > > a wrapper for `popen` with `os::shell`. It's used a handful of places in > > the code instead of `popen`: > > > > ostringstream out; > > > > Try<int> status = os::shell(&out, ip_discovery_script.get()); > > > > if (status.isError()) { > > EXIT(EXIT_FAILURE) << ...; > > } else if (WIFSIGNALED(status.get())) { > > EXIT(EXIT_FAILURE) << ... << strsignal(WTERMSIG(status.get())); > > } else if (WEXITSTATUS(status.get()) != EXIT_SUCCESS) { > > EXIT(EXIT_FAILURE) << ... << stringify > > (WEXITSTATUS(status.get())); > > } > > > > const string ipAddress = strings::trim(out.get()); > > > > LOG(INFO) << ...; > > > > os::setenv("LIBPROCESS_IP", ipAddress); LoL - no one seemed to know about it :) I asked in the 'core' slack and was pointed to at least five different ways of doing this. Thanks! > On July 19, 2015, 8:09 p.m., Benjamin Hindman wrote: > > src/master/main.cpp, line 184 > > <https://reviews.apache.org/r/36425/diff/4/?file=1010039#file1010039line184> > > > > What is process::executeCommand? Did you mean process::subprocess here? it's the method I'm introducing in the patch immediately before this: https://reviews.apache.org/r/36424/ - Marco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36425/#review92219 ----------------------------------------------------------- On July 13, 2015, 9:35 p.m., Marco Massenzio wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36425/ > ----------------------------------------------------------- > > (Updated July 13, 2015, 9:35 p.m.) > > > Review request for mesos, Benjamin Hindman and Cody Maloney. > > > Bugs: MESOS-2902 > https://issues.apache.org/jira/browse/MESOS-2902 > > > Repository: mesos > > > Description > ------- > > Jira: MESOS-2902 > > It is sometimes useful to enable an external script to > configure the IP address the Mesos Master will bind to > on the server, where it's not desirable to set the > --ip flag and/or a "wrapper" script is not a viable option. > > This patch adds a --ip_discovery_script to point to a local > script that will emit as its only output the IP address that > the Master will bind to: only spaces and newlines are allowed; > further, as we cannot use the `libprocess` sub-processing > facilities, we cannot timeout the script, should this block > for long times (or even forever). > > This will override the --ip flag, which, even if set, will be > ignored. > > > Diffs > ----- > > docs/configuration.md feee5594c88112f77ce382cb3dd8628653f92d01 > src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 > > Diff: https://reviews.apache.org/r/36425/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Marco Massenzio > >
