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

2015-12-18 Thread Guangya Liu


> On 十二月 19, 2015, 3:58 a.m., Guangya Liu wrote:
> > docs/roles.md, line 29
> > 
> >
> > 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.

2015-12-18 Thread Neil Conway


> On Dec. 19, 2015, 3:58 a.m., Guangya Liu wrote:
> > docs/roles.md, line 29
> > 
> >
> > 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.

2015-12-18 Thread Guangya Liu

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



docs/roles.md (line 29)


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.

2015-12-18 Thread Adam B

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

2015-12-18 Thread Neil Conway

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

2015-12-18 Thread Neil Conway


> On Dec. 18, 2015, 6:25 a.m., Adam B wrote:
> > src/master/flags.cpp, lines 187-189
> > 
> >
> > 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/#review22
---


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.

2015-12-17 Thread Adam B

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


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


docs/upgrades.md (line 11)


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)


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



docs/upgrades.md (line 18)


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



src/master/flags.cpp (lines 186 - 188)


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.

2015-12-15 Thread Mesos ReviewBot

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