Re: Review Request 36294: Updated configuration.md with new flags for 0.23.0.

2015-07-08 Thread Marco Massenzio


> 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.

2015-07-08 Thread Adam B


> 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.

2015-07-08 Thread Jie Yu


> On July 8, 2015, 5:24 p.m., Jie Yu wrote:
> > docs/configuration.md, lines 1295-1301
> > 
> >
> > 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.

2015-07-08 Thread Adam B


> On July 8, 2015, 10:24 a.m., Jie Yu wrote:
> > docs/configuration.md, lines 1295-1301
> > 
> >
> > 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.

2015-07-08 Thread Adam B


> On July 8, 2015, 9:35 a.m., Marco Massenzio wrote:
> > docs/configuration.md, line 1031
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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.

2015-07-08 Thread Jie Yu

---
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)


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.

2015-07-08 Thread Marco Massenzio

---
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)


nit: Google (uppercase)



docs/configuration.md (line 91)


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)


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.

2015-07-08 Thread Adam B

---
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.

2015-07-08 Thread Mesos ReviewBot

---
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
> 
>



Review Request 36294: Updated configuration.md with new flags for 0.23.0.

2015-07-08 Thread Adam B

---
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