You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Adam B <ad...@mesosphere.io> on 2015/07/08 09:19:22 UTC
Review Request 36294: Updated configuration.md with new flags for
0.23.0.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36294/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Joris Van Remoortere, Marco Massenzio, and Vinod Kone.
Bugs: MESOS-2727
https://issues.apache.org/jira/browse/MESOS-2727
Repository: mesos-incubating
Description
-------
See Summary.
Diffs
-----
docs/configuration.md 9a8505d
Diff: https://reviews.apache.org/r/36294/diff/
Testing
-------
I went through each of the flags.hpp's and made sure each flag was represented.
I did not go so far as to check for text rewordings.
Some of you Shepherded/committed new flags in 0.23.0 without any corresponding configuration.md update.
Let's try to make sure updating configuration.md is included as a part of whatever feature work introduces the new flag, so that future release managers don't have to document your feature for you.
Thanks,
Adam B
Re: Review Request 36294: Updated configuration.md with new flags for
0.23.0.
Posted by Marco Massenzio <ma...@mesosphere.io>.
> On July 8, 2015, 4:35 p.m., Marco Massenzio wrote:
> > Thanks for doing this!
> >
> > It seems to me that this is just a 'prettified' form of the `--help` output? perhaps we could automate its generation, removing the need of manually double-checking.
> > Just a thought.
>
> Adam B wrote:
> +1000, created https://issues.apache.org/jira/browse/MESOS-3019
LoL - you beat me to it :)
(looks like a fun weekend python project, I may get round to do it myself)
thanks!
- Marco
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36294/#review90913
-----------------------------------------------------------
On July 8, 2015, 8:18 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36294/
> -----------------------------------------------------------
>
> (Updated July 8, 2015, 8:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Joris Van Remoortere, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2727
> https://issues.apache.org/jira/browse/MESOS-2727
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> See Summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 9a8505d
>
> Diff: https://reviews.apache.org/r/36294/diff/
>
>
> Testing
> -------
>
> I went through each of the flags.hpp's and made sure each flag was represented.
> I did not go so far as to check for text rewordings.
>
> Rendered at https://www.notehub.org/2015/7/8/mesos-configuration
>
> Some of you Shepherded/committed new flags in 0.23.0 without any corresponding configuration.md update.
> Let's try to make sure updating configuration.md is included as a part of whatever feature work introduces the new flag, so that future release managers don't have to document your feature for you.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 36294: Updated configuration.md with new flags for
0.23.0.
Posted by Adam B <ad...@mesosphere.io>.
> On July 8, 2015, 9:35 a.m., Marco Massenzio wrote:
> > Thanks for doing this!
> >
> > It seems to me that this is just a 'prettified' form of the `--help` output? perhaps we could automate its generation, removing the need of manually double-checking.
> > Just a thought.
+1000, created https://issues.apache.org/jira/browse/MESOS-3019
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36294/#review90913
-----------------------------------------------------------
On July 8, 2015, 1:18 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36294/
> -----------------------------------------------------------
>
> (Updated July 8, 2015, 1:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Joris Van Remoortere, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2727
> https://issues.apache.org/jira/browse/MESOS-2727
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> See Summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 9a8505d
>
> Diff: https://reviews.apache.org/r/36294/diff/
>
>
> Testing
> -------
>
> I went through each of the flags.hpp's and made sure each flag was represented.
> I did not go so far as to check for text rewordings.
>
> Rendered at https://www.notehub.org/2015/7/8/mesos-configuration
>
> Some of you Shepherded/committed new flags in 0.23.0 without any corresponding configuration.md update.
> Let's try to make sure updating configuration.md is included as a part of whatever feature work introduces the new flag, so that future release managers don't have to document your feature for you.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 36294: Updated configuration.md with new flags for
0.23.0.
Posted by Adam B <ad...@mesosphere.io>.
> On July 8, 2015, 9:35 a.m., Marco Massenzio wrote:
> > docs/configuration.md, line 1031
> > <https://reviews.apache.org/r/36294/diff/1/?file=1001995#file1001995line1031>
> >
> > it would be awesome if we could have an exhaustive list of allowed time units: eg, is msecs or usecs allowed? what about `days`?
> >
> > and, I presume only ints are allowed, but it would be nice to make it clear (eg, `0.5mins` is not allowed).
> >
> > also, one has to *infer* that the singular form is *not* allowed (`1mins` in the default) but it would be nice to spell it out for the unwary
What? Isn't it obvious from https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp#L51 what units are allowed? ;P
Also, doubles are allowed, but no negative values (yet). https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp#L38
> On July 8, 2015, 9:35 a.m., Marco Massenzio wrote:
> > docs/configuration.md, line 91
> > <https://reviews.apache.org/r/36294/diff/1/?file=1001995#file1001995line91>
> >
> > unnecessary blank line
> > (and I would add a simple example, and state that this has to actually be an IP address, can't just be a DNS name) and should have no port info.
These blank lines are scattered through several flags, so I'll leave them alone for now.
As for an IP example, I was really just moving this flag into alpha-order. The real change needs to happen in master/main.cpp and slave/main.cpp.
> On July 8, 2015, 9:35 a.m., Marco Massenzio wrote:
> > docs/configuration.md, line 81
> > <https://reviews.apache.org/r/36294/diff/1/?file=1001995#file1001995line81>
> >
> > nit: Google (uppercase)
Fixed this in the configuration and in src/logging/flags.cpp
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36294/#review90913
-----------------------------------------------------------
On July 8, 2015, 1:18 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36294/
> -----------------------------------------------------------
>
> (Updated July 8, 2015, 1:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Joris Van Remoortere, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2727
> https://issues.apache.org/jira/browse/MESOS-2727
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> See Summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 9a8505d
>
> Diff: https://reviews.apache.org/r/36294/diff/
>
>
> Testing
> -------
>
> I went through each of the flags.hpp's and made sure each flag was represented.
> I did not go so far as to check for text rewordings.
>
> Rendered at https://www.notehub.org/2015/7/8/mesos-configuration
>
> Some of you Shepherded/committed new flags in 0.23.0 without any corresponding configuration.md update.
> Let's try to make sure updating configuration.md is included as a part of whatever feature work introduces the new flag, so that future release managers don't have to document your feature for you.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 36294: Updated configuration.md with new flags for
0.23.0.
Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36294/#review90913
-----------------------------------------------------------
Ship it!
Thanks for doing this!
It seems to me that this is just a 'prettified' form of the `--help` output? perhaps we could automate its generation, removing the need of manually double-checking.
Just a thought.
docs/configuration.md (line 81)
<https://reviews.apache.org/r/36294/#comment144083>
nit: Google (uppercase)
docs/configuration.md (line 91)
<https://reviews.apache.org/r/36294/#comment144086>
unnecessary blank line
(and I would add a simple example, and state that this has to actually be an IP address, can't just be a DNS name) and should have no port info.
docs/configuration.md (line 1021)
<https://reviews.apache.org/r/36294/#comment144088>
it would be awesome if we could have an exhaustive list of allowed time units: eg, is msecs or usecs allowed? what about `days`?
and, I presume only ints are allowed, but it would be nice to make it clear (eg, `0.5mins` is not allowed).
also, one has to *infer* that the singular form is *not* allowed (`1mins` in the default) but it would be nice to spell it out for the unwary
- Marco Massenzio
On July 8, 2015, 8:18 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36294/
> -----------------------------------------------------------
>
> (Updated July 8, 2015, 8:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Joris Van Remoortere, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2727
> https://issues.apache.org/jira/browse/MESOS-2727
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> See Summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 9a8505d
>
> Diff: https://reviews.apache.org/r/36294/diff/
>
>
> Testing
> -------
>
> I went through each of the flags.hpp's and made sure each flag was represented.
> I did not go so far as to check for text rewordings.
>
> Rendered at https://www.notehub.org/2015/7/8/mesos-configuration
>
> Some of you Shepherded/committed new flags in 0.23.0 without any corresponding configuration.md update.
> Let's try to make sure updating configuration.md is included as a part of whatever feature work introduces the new flag, so that future release managers don't have to document your feature for you.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 36294: Updated configuration.md with new flags for
0.23.0.
Posted by Adam B <ad...@mesosphere.io>.
> On July 8, 2015, 10:24 a.m., Jie Yu wrote:
> > docs/configuration.md, lines 1295-1301
> > <https://reviews.apache.org/r/36294/diff/1/?file=1001995#file1001995line1295>
> >
> > This one is being deprecated. Maybe you want to call it out?
Shouldn't we also call that out in the Flags help in src/slave/flags.cpp?
That seems more important for those already using the flag. If we're actually deprecating it, we should be printing a warning message somewhere if it's used. And it should go into the Deprecation section of the CHANGELOG. As it is, I don't even see this flag being used anywhere (maybe my grep-fu is off).
I'm going to leave this one alone for now. If somebody wants to officially deprecate this flag, they can submit a separate patch.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36294/#review90927
-----------------------------------------------------------
On July 8, 2015, 1:18 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36294/
> -----------------------------------------------------------
>
> (Updated July 8, 2015, 1:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Joris Van Remoortere, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2727
> https://issues.apache.org/jira/browse/MESOS-2727
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> See Summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 9a8505d
>
> Diff: https://reviews.apache.org/r/36294/diff/
>
>
> Testing
> -------
>
> I went through each of the flags.hpp's and made sure each flag was represented.
> I did not go so far as to check for text rewordings.
>
> Rendered at https://www.notehub.org/2015/7/8/mesos-configuration
>
> Some of you Shepherded/committed new flags in 0.23.0 without any corresponding configuration.md update.
> Let's try to make sure updating configuration.md is included as a part of whatever feature work introduces the new flag, so that future release managers don't have to document your feature for you.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 36294: Updated configuration.md with new flags for
0.23.0.
Posted by Jie Yu <yu...@gmail.com>.
> On July 8, 2015, 5:24 p.m., Jie Yu wrote:
> > docs/configuration.md, lines 1295-1301
> > <https://reviews.apache.org/r/36294/diff/1/?file=1001995#file1001995line1295>
> >
> > This one is being deprecated. Maybe you want to call it out?
>
> Adam B wrote:
> Shouldn't we also call that out in the Flags help in src/slave/flags.cpp?
> That seems more important for those already using the flag. If we're actually deprecating it, we should be printing a warning message somewhere if it's used. And it should go into the Deprecation section of the CHANGELOG. As it is, I don't even see this flag being used anywhere (maybe my grep-fu is off).
> I'm going to leave this one alone for now. If somebody wants to officially deprecate this flag, they can submit a separate patch.
I think Niklas added the NOTE about the deprecation in the slave/flags.hpp. cc Niklas
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36294/#review90927
-----------------------------------------------------------
On July 8, 2015, 8:18 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36294/
> -----------------------------------------------------------
>
> (Updated July 8, 2015, 8:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Joris Van Remoortere, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2727
> https://issues.apache.org/jira/browse/MESOS-2727
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> See Summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 9a8505d
>
> Diff: https://reviews.apache.org/r/36294/diff/
>
>
> Testing
> -------
>
> I went through each of the flags.hpp's and made sure each flag was represented.
> I did not go so far as to check for text rewordings.
>
> Rendered at https://www.notehub.org/2015/7/8/mesos-configuration
>
> Some of you Shepherded/committed new flags in 0.23.0 without any corresponding configuration.md update.
> Let's try to make sure updating configuration.md is included as a part of whatever feature work introduces the new flag, so that future release managers don't have to document your feature for you.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 36294: Updated configuration.md with new flags for
0.23.0.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36294/#review90927
-----------------------------------------------------------
Ship it!
Thanks Adam! I agree with Marco that this probably should be automated in the future:)
docs/configuration.md (lines 1284 - 1290)
<https://reviews.apache.org/r/36294/#comment144094>
This one is being deprecated. Maybe you want to call it out?
- Jie Yu
On July 8, 2015, 8:18 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36294/
> -----------------------------------------------------------
>
> (Updated July 8, 2015, 8:18 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Joris Van Remoortere, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2727
> https://issues.apache.org/jira/browse/MESOS-2727
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> See Summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 9a8505d
>
> Diff: https://reviews.apache.org/r/36294/diff/
>
>
> Testing
> -------
>
> I went through each of the flags.hpp's and made sure each flag was represented.
> I did not go so far as to check for text rewordings.
>
> Rendered at https://www.notehub.org/2015/7/8/mesos-configuration
>
> Some of you Shepherded/committed new flags in 0.23.0 without any corresponding configuration.md update.
> Let's try to make sure updating configuration.md is included as a part of whatever feature work introduces the new flag, so that future release managers don't have to document your feature for you.
>
>
> Thanks,
>
> Adam B
>
>
Re: Review Request 36294: Updated configuration.md with new flags for
0.23.0.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36294/
-----------------------------------------------------------
(Updated July 8, 2015, 1:18 a.m.)
Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Joris Van Remoortere, Marco Massenzio, and Vinod Kone.
Bugs: MESOS-2727
https://issues.apache.org/jira/browse/MESOS-2727
Repository: mesos-incubating
Description
-------
See Summary.
Diffs
-----
docs/configuration.md 9a8505d
Diff: https://reviews.apache.org/r/36294/diff/
Testing (updated)
-------
I went through each of the flags.hpp's and made sure each flag was represented.
I did not go so far as to check for text rewordings.
Rendered at https://www.notehub.org/2015/7/8/mesos-configuration
Some of you Shepherded/committed new flags in 0.23.0 without any corresponding configuration.md update.
Let's try to make sure updating configuration.md is included as a part of whatever feature work introduces the new flag, so that future release managers don't have to document your feature for you.
Thanks,
Adam B
Re: Review Request 36294: Updated configuration.md with new flags for
0.23.0.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36294/#review90875
-----------------------------------------------------------
Patch looks great!
Reviews applied: [36294]
All tests passed.
- Mesos ReviewBot
On July 8, 2015, 7:19 a.m., Adam B wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36294/
> -----------------------------------------------------------
>
> (Updated July 8, 2015, 7:19 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, Joris Van Remoortere, Marco Massenzio, and Vinod Kone.
>
>
> Bugs: MESOS-2727
> https://issues.apache.org/jira/browse/MESOS-2727
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> See Summary.
>
>
> Diffs
> -----
>
> docs/configuration.md 9a8505d
>
> Diff: https://reviews.apache.org/r/36294/diff/
>
>
> Testing
> -------
>
> I went through each of the flags.hpp's and made sure each flag was represented.
> I did not go so far as to check for text rewordings.
>
> Some of you Shepherded/committed new flags in 0.23.0 without any corresponding configuration.md update.
> Let's try to make sure updating configuration.md is included as a part of whatever feature work introduces the new flag, so that future release managers don't have to document your feature for you.
>
>
> Thanks,
>
> Adam B
>
>