You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Klaus Ma <kl...@cguru.net> on 2015/09/10 17:47:13 UTC
Review Request 38259: [MESOS-3340] Command-line flags should take
precedence over OS Env variables
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38259/
-----------------------------------------------------------
Review request for mesos and Bernd Mathiske.
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 9da213f802aec6a7768ce6f5aea7b437d980356a
3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp ebf8cd656625b7fd414cacaa87f156c95df29438
Diff: https://reviews.apache.org/r/38259/diff/
Testing
-------
make
make check
Thanks,
Klaus Ma
Re: Review Request 38259: [MESOS-3340] Command-line flags should take
precedence over OS Env variables
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38259/#review98413
-----------------------------------------------------------
Patch looks great!
Reviews applied: [38259]
All tests passed.
- Mesos ReviewBot
On Sept. 10, 2015, 3:47 p.m., Klaus Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38259/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2015, 3:47 p.m.)
>
>
> Review request for mesos and Bernd Mathiske.
>
>
> 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 9da213f802aec6a7768ce6f5aea7b437d980356a
> 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp ebf8cd656625b7fd414cacaa87f156c95df29438
>
> Diff: https://reviews.apache.org/r/38259/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Klaus Ma
>
>
Re: Review Request 38259: [MESOS-3340] Command-line flags should take
precedence over OS Env variables
Posted by haosdent huang <ha...@gmail.com>.
> On Sept. 10, 2015, 4:40 p.m., haosdent huang wrote:
> > LGTM. But I not sure if this match @marco expect. Could you add he as reviewers? So currently your approach is show error message when `duplicates` is true. Use command line parameters when command line variables conflict with environment variables. I also suggest add comment and test cases about the last condition.
```
Use command line parameters when command line variables conflict with environment variables.
```
->
```
When duplicates is false, use command line parameters while ignore environment variables.
```
- haosdent
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38259/#review98410
-----------------------------------------------------------
On Sept. 10, 2015, 3:47 p.m., Klaus Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38259/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2015, 3:47 p.m.)
>
>
> Review request for mesos and Bernd Mathiske.
>
>
> 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 9da213f802aec6a7768ce6f5aea7b437d980356a
> 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp ebf8cd656625b7fd414cacaa87f156c95df29438
>
> Diff: https://reviews.apache.org/r/38259/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Klaus Ma
>
>
Re: Review Request 38259: [MESOS-3340] Command-line flags should take
precedence over OS Env variables
Posted by Klaus Ma <kl...@cguru.net>.
> On Sept. 10, 2015, 4:40 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, line 569
> > <https://reviews.apache.org/r/38259/diff/1/?file=1067198#file1067198line569>
> >
> > Is it possible to change to foreach here?
>
> Klaus Ma wrote:
> OK, let me address it.
Just checked the code, we can not do that because 1) it's an array, 2) no requirement that argv is ended with NULL.
- Klaus
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38259/#review98410
-----------------------------------------------------------
On Sept. 11, 2015, 2:43 a.m., Klaus Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38259/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2015, 2:43 a.m.)
>
>
> Review request for mesos, Bernd Mathiske and Marco Massenzio.
>
>
> 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 9da213f802aec6a7768ce6f5aea7b437d980356a
> 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp ebf8cd656625b7fd414cacaa87f156c95df29438
>
> Diff: https://reviews.apache.org/r/38259/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Klaus Ma
>
>
Re: Review Request 38259: [MESOS-3340] Command-line flags should take
precedence over OS Env variables
Posted by Klaus Ma <kl...@cguru.net>.
> On Sept. 10, 2015, 4:40 p.m., haosdent huang wrote:
> > LGTM. But I not sure if this match @marco expect. Could you add he as reviewers? So currently your approach is show error message when `duplicates` is true. Use command line parameters when command line variables conflict with environment variables. I also suggest add comment and test cases about the last condition.
>
> haosdent huang wrote:
> ```
> Use command line parameters when command line variables conflict with environment variables.
> ```
> ->
> ```
> When duplicates is false, use command line parameters while ignore environment variables.
> ```
Add Macro as reviewer. Current solution is to throw error message and abort the command line if any duplicated items, so uer'll not be confused by overwrite.
> On Sept. 10, 2015, 4:40 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, line 600
> > <https://reviews.apache.org/r/38259/diff/1/?file=1067198#file1067198line600>
> >
> > The style is not correct here. Please change to
> > ```
> > return Error(
> > "Duplicate flag '" + name "' in command line with environment");
> > ```
Sure, I'll correct it :).
> On Sept. 10, 2015, 4:40 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, line 569
> > <https://reviews.apache.org/r/38259/diff/1/?file=1067198#file1067198line569>
> >
> > Is it possible to change to foreach here?
OK, let me address it.
- Klaus
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38259/#review98410
-----------------------------------------------------------
On Sept. 11, 2015, 2:43 a.m., Klaus Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38259/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2015, 2:43 a.m.)
>
>
> Review request for mesos, Bernd Mathiske and Marco Massenzio.
>
>
> 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 9da213f802aec6a7768ce6f5aea7b437d980356a
> 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp ebf8cd656625b7fd414cacaa87f156c95df29438
>
> Diff: https://reviews.apache.org/r/38259/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Klaus Ma
>
>
Re: Review Request 38259: [MESOS-3340] Command-line flags should take
precedence over OS Env variables
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38259/#review98410
-----------------------------------------------------------
LGTM. But I not sure if this match @marco expect. Could you add he as reviewers? So currently your approach is show error message when `duplicates` is true. Use command line parameters when command line variables conflict with environment variables. I also suggest add comment and test cases about the last condition.
3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (line 569)
<https://reviews.apache.org/r/38259/#comment154880>
Is it possible to change to foreach here?
3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (line 600)
<https://reviews.apache.org/r/38259/#comment154879>
The style is not correct here. Please change to
```
return Error(
"Duplicate flag '" + name "' in command line with environment");
```
3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (line 675)
<https://reviews.apache.org/r/38259/#comment154881>
Ditto
- haosdent huang
On Sept. 10, 2015, 3:47 p.m., Klaus Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38259/
> -----------------------------------------------------------
>
> (Updated Sept. 10, 2015, 3:47 p.m.)
>
>
> Review request for mesos and Bernd Mathiske.
>
>
> 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 9da213f802aec6a7768ce6f5aea7b437d980356a
> 3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp ebf8cd656625b7fd414cacaa87f156c95df29438
>
> Diff: https://reviews.apache.org/r/38259/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Klaus Ma
>
>