You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Neil Conway <ne...@gmail.com> on 2015/12/15 21:59:19 UTC

Review Request 41408: Updated documentation for implicit roles.

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

Review request for mesos, Adam B and Yongqiao Wang.


Bugs: MESOS-4085
    https://issues.apache.org/jira/browse/MESOS-4085


Repository: mesos


Description
-------

Updated documentation for implicit roles.


Diffs
-----

  docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
  docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
  docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
  docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 

Diff: https://reviews.apache.org/r/41408/diff/


Testing
-------


Thanks,

Neil Conway


Re: Review Request 41408: Updated documentation for implicit roles.

Posted by Neil Conway <ne...@gmail.com>.

> On Dec. 19, 2015, 3:58 a.m., Guangya Liu wrote:
> > docs/roles.md, line 29
> > <https://reviews.apache.org/r/41408/diff/2/?file=1171263#file1171263line29>
> >
> >     One minor comment: The whitelist is already a keyword when mesos master star up, not sure if this may bring some confusion for end users, what about using _role list_ instead of _role whitelist_?

I'd vote we stick with "role whitelist"; the other common usage in Mesos is a "slave whitelist", and so it should be clear by context (as well as the use of "role" vs. "slave" + whitelist) which one is being referred to.


- Neil


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


On Dec. 18, 2015, 8 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41408/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Bugs: MESOS-4085
>     https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated documentation for implicit roles.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 388b8b91df6fce826413649b0a1480af3f78e918 
>   docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
>   docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
> 
> Diff: https://reviews.apache.org/r/41408/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 41408: Updated documentation for implicit roles.

Posted by Guangya Liu <gy...@gmail.com>.

> On 十二月 19, 2015, 3:58 a.m., Guangya Liu wrote:
> > docs/roles.md, line 29
> > <https://reviews.apache.org/r/41408/diff/2/?file=1171263#file1171263line29>
> >
> >     One minor comment: The whitelist is already a keyword when mesos master star up, not sure if this may bring some confusion for end users, what about using _role list_ instead of _role whitelist_?
> 
> Neil Conway wrote:
>     I'd vote we stick with "role whitelist"; the other common usage in Mesos is a "slave whitelist", and so it should be clear by context (as well as the use of "role" vs. "slave" + whitelist) which one is being referred to.

Yes, but the difference is that the "slave/agent whitelist" is using "whitelist" as flag (https://github.com/apache/mesos/blob/master/src/master/flags.cpp#L153-L157), but "role whitelist" is using "role" as flag. When I first read this document, I was a bit confused with _role whitelist_, because I was always thinking _whitelist_ is "slave whitelist". ;-) But after finish the document, I was clear about _role whitelist_. It depends on you to make the decision because I did not mark this as an issue but only a question. Thanks.


- Guangya


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


On 十二月 18, 2015, 8 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41408/
> -----------------------------------------------------------
> 
> (Updated 十二月 18, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Bugs: MESOS-4085
>     https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated documentation for implicit roles.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 388b8b91df6fce826413649b0a1480af3f78e918 
>   docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
>   docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
> 
> Diff: https://reviews.apache.org/r/41408/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 41408: Updated documentation for implicit roles.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41408/#review111356
-----------------------------------------------------------



docs/roles.md (line 29)
<https://reviews.apache.org/r/41408/#comment171518>

    One minor comment: The whitelist is already a keyword when mesos master star up, not sure if this may bring some confusion for end users, what about using _role list_ instead of _role whitelist_?


- Guangya Liu


On 十二月 18, 2015, 8 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41408/
> -----------------------------------------------------------
> 
> (Updated 十二月 18, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Bugs: MESOS-4085
>     https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated documentation for implicit roles.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 388b8b91df6fce826413649b0a1480af3f78e918 
>   docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
>   docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
> 
> Diff: https://reviews.apache.org/r/41408/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 41408: Updated documentation for implicit roles.

Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41408/#review111312
-----------------------------------------------------------

Ship it!


Ship It!

- Adam B


On Dec. 18, 2015, noon, Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41408/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2015, noon)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Bugs: MESOS-4085
>     https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated documentation for implicit roles.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 388b8b91df6fce826413649b0a1480af3f78e918 
>   docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
>   docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
> 
> Diff: https://reviews.apache.org/r/41408/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 41408: Updated documentation for implicit roles.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41408/
-----------------------------------------------------------

(Updated Dec. 18, 2015, 8 p.m.)


Review request for mesos, Adam B and Yongqiao Wang.


Changes
-------

Address review comments.


Bugs: MESOS-4085
    https://issues.apache.org/jira/browse/MESOS-4085


Repository: mesos


Description
-------

Updated documentation for implicit roles.


Diffs (updated)
-----

  CHANGELOG 388b8b91df6fce826413649b0a1480af3f78e918 
  docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
  docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
  docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
  docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 

Diff: https://reviews.apache.org/r/41408/diff/


Testing
-------


Thanks,

Neil Conway


Re: Review Request 41408: Updated documentation for implicit roles.

Posted by Neil Conway <ne...@gmail.com>.

> On Dec. 18, 2015, 6:25 a.m., Adam B wrote:
> > src/master/flags.cpp, lines 187-189
> > <https://reviews.apache.org/r/41408/diff/1/?file=1165155#file1165155line187>
> >
> >     Don't forget to update configuration.md to match your change to the flags.

This was already done as far as I can tell.


- Neil


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


On Dec. 15, 2015, 8:59 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41408/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2015, 8:59 p.m.)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Bugs: MESOS-4085
>     https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated documentation for implicit roles.
> 
> 
> Diffs
> -----
> 
>   docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
>   docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
> 
> Diff: https://reviews.apache.org/r/41408/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 41408: Updated documentation for implicit roles.

Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41408/#review111122
-----------------------------------------------------------


Looks good, but please also update the CHANGELOG and configuration.md


docs/upgrades.md (line 11)
<https://reviews.apache.org/r/41408/#comment171114>

    Let's call out this JIRA(s) in the API changes section in the 0.27 CHANGELOG as well.



docs/upgrades.md (lines 12 - 13)
<https://reviews.apache.org/r/41408/#comment171117>

    "must be specified" - if you want to use roles. You can leave it out, but everything goes to "*"



docs/upgrades.md (line 18)
<https://reviews.apache.org/r/41408/#comment171111>

    Link to a JIRA, diff, or other doc that explains the API change in more detail.



src/master/flags.cpp (lines 186 - 188)
<https://reviews.apache.org/r/41408/#comment171109>

    Don't forget to update configuration.md to match your change to the flags.


- Adam B


On Dec. 15, 2015, 12:59 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41408/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2015, 12:59 p.m.)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Bugs: MESOS-4085
>     https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated documentation for implicit roles.
> 
> 
> Diffs
> -----
> 
>   docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
>   docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
> 
> Diff: https://reviews.apache.org/r/41408/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


Re: Review Request 41408: Updated documentation for implicit roles.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41408/#review110575
-----------------------------------------------------------


Patch looks great!

Reviews applied: [40995, 41075, 41225, 41408]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 15, 2015, 8:59 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41408/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2015, 8:59 p.m.)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Bugs: MESOS-4085
>     https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated documentation for implicit roles.
> 
> 
> Diffs
> -----
> 
>   docs/attributes-resources.md 3c5f2776d44a050ee3ac9967dd0ba0253b9c4558 
>   docs/configuration.md c75f56ce849f6960b3b6246bfa6949156a82eabb 
>   docs/roles.md 459d70717f4a0bdd921bb18b6ddc2505b6fc596e 
>   docs/upgrades.md 7c1f1814680078380ca33bbc27421675ffe61d60 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
> 
> Diff: https://reviews.apache.org/r/41408/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Neil Conway
> 
>