mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Klaus Ma" <kl...@cguru.net>
Subject Re: Review Request 38259: [MESOS-3340] Command-line flags should take precedence over OS Env variables
Date Sun, 13 Sep 2015 16:20:55 GMT


> On Sept. 11, 2015, 6:56 a.m., Marco Massenzio wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, lines 600-601
> > <https://reviews.apache.org/r/38259/diff/2/?file=1068027#file1068027line600>
> >
> >     the `+` should be at the end of the line (before the newline).
> >     Can you also please change the error message to:
> >     ```
> >     "Command line flag '" + name +
> >     " duplicates environment variable definition with the same name"
> >     ```
> >     also, please feel free to use all 80 columns.
> >     
> >     Now, an interesting question (for the shepherd/committer to decide) is whether
we should just emit a WARN and have the command line (explicit) setting override the OS Env
(implicit).
> >     
> >     Also, whether we should only WARN, but proceed, if the **values** are the same:
> >     
> >     ```MESOS_IP=127.0.0.1
> >     ... --ip=127.0.0.1
> >     ```
> >     could (arguably) be considered a valid setting.
> >     
> >     As a general approach, I'd suggest however to stick with current behavior, to
avoid breaking stuff that is currently working just fine (and relying on the executable to
error out in case of misconfigurations).
> 
> Klaus Ma wrote:
>     I used @haosdent's suggestion as follow, it looks better than mine :).
>     
>         return Error(
>             "Duplicate flag '" + name "' in command line with environment");
>     
>     Your question is interesting, and I think it's a great improvement. Agree with you
to keep current behavior, we can log a new ticket to trace that question.

The behavior was updated to over-write environment by command line.


- Klaus


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38259/#review98568
-----------------------------------------------------------


On Sept. 13, 2015, 5:30 a.m., Klaus Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38259/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2015, 5:30 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Marco Massenzio, and Michael Park.
> 
> 
> Bugs: MESOS-3340
>     https://issues.apache.org/jira/browse/MESOS-3340
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently, it appears that re-defining a flag on the command-line that was already defined
via a OS Env var (MESOS_*) causes the Master to fail with a not very helpful message.
> 
> For example, if one has MESOS_QUORUM defined, this happens:
> 
>     $ ./mesos-master --zk=zk://192.168.1.4/mesos --quorum=1 --hostname=192.168.1.4 --ip=192.168.1.4
>     Duplicate flag 'quorum' on command line
> 
> which is not very helpful.
> 
> Current solution is to throw error if any duplication; over-write may make user confused
about the result.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 9da213f 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp ebf8cd6 
> 
> Diff: https://reviews.apache.org/r/38259/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message