You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rojas <al...@mesosphere.io> on 2017/04/04 09:22:28 UTC
Re: Review Request 57474: Added test for authorization of
hierarchical roles.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57474/
-----------------------------------------------------------
(Updated April 4, 2017, 11:22 a.m.)
Review request for mesos, Adam B and Benjamin Bannier.
Changes
-------
Rebased.
Repository: mesos
Description
-------
Adds tests for each of the actions which support hierarchical roles.
Diffs (updated)
-----
src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d
Diff: https://reviews.apache.org/r/57474/diff/4/
Changes: https://reviews.apache.org/r/57474/diff/3-4/
Testing
-------
`make check`
Thanks,
Alexander Rojas
Re: Review Request 57474: Added test for authorization of
hierarchical roles.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57474/#review171690
-----------------------------------------------------------
Test logic looks good, although I question "archbishop" as a principal.
And it doesn't look like you thoroughly reviewed the text in your comments.
src/tests/authorization_tests.cpp
Lines 888 (patched)
<https://reviews.apache.org/r/57474/#comment244705>
s/Cheks/Checks/
src/tests/authorization_tests.cpp
Lines 912 (patched)
<https://reviews.apache.org/r/57474/#comment244706>
When has an archbishop ever also been a king? I was expecting another person's name. As a title, "archbishop" makes more sense as a Mesos role than a principal.
Archbishop William King was technically both.
https://en.wikipedia.org/wiki/William_King_(bishop)
src/tests/authorization_tests.cpp
Lines 1048 (patched)
<https://reviews.apache.org/r/57474/#comment244708>
s/Not not/Not/
s/irself/itself/
src/tests/authorization_tests.cpp
Lines 1184 (patched)
<https://reviews.apache.org/r/57474/#comment244709>
s/register frameworks/reserve resources/
src/tests/authorization_tests.cpp
Lines 1478 (patched)
<https://reviews.apache.org/r/57474/#comment244710>
"Not not" and "irself" again. Please fix throughout.
src/tests/authorization_tests.cpp
Lines 1597 (patched)
<https://reviews.apache.org/r/57474/#comment244711>
s/register frameworks/create volumes/
src/tests/authorization_tests.cpp
Lines 1866 (patched)
<https://reviews.apache.org/r/57474/#comment244712>
These could uses comments like the others
src/tests/authorization_tests.cpp
Line 1578 (original), 1943 (patched)
<https://reviews.apache.org/r/57474/#comment244714>
"register frameworks" again. Please fix throughout.
src/tests/authorization_tests.cpp
Lines 4522-4524 (patched)
<https://reviews.apache.org/r/57474/#comment244715>
This comment belongs above the previous block of archbishop->king
src/tests/authorization_tests.cpp
Lines 4588 (patched)
<https://reviews.apache.org/r/57474/#comment244719>
s/view any role/update the weight of any role/
src/tests/authorization_tests.cpp
Lines 4737 (patched)
<https://reviews.apache.org/r/57474/#comment244716>
s/get weights/get quotas/g and s/update weights/get quotas/g throughout this test.
src/tests/authorization_tests.cpp
Lines 4753 (patched)
<https://reviews.apache.org/r/57474/#comment244718>
s/update the weight/view the quota/
src/tests/authorization_tests.cpp
Lines 4760 (patched)
<https://reviews.apache.org/r/57474/#comment244717>
s/view any role/view quota for any role/
- Adam B
On April 11, 2017, 1:47 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57474/
> -----------------------------------------------------------
>
> (Updated April 11, 2017, 1:47 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Bannier.
>
>
> Bugs: MESOS-7026
> https://issues.apache.org/jira/browse/MESOS-7026
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds tests for each of the actions which support hierarchical roles.
>
>
> Diffs
> -----
>
> src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d
>
>
> Diff: https://reviews.apache.org/r/57474/diff/4/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 57474: Added test for authorization of
hierarchical roles.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57474/#review172202
-----------------------------------------------------------
Bad review!
Reviews applied: [57474, 57473, 58292, 57166, 56805, 57165, 57164]
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos Reviewbot
On April 18, 2017, 1:28 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57474/
> -----------------------------------------------------------
>
> (Updated April 18, 2017, 1:28 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Bannier.
>
>
> Bugs: MESOS-7026
> https://issues.apache.org/jira/browse/MESOS-7026
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds tests for each of the actions which support hierarchical roles.
>
>
> Diffs
> -----
>
> src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d
>
>
> Diff: https://reviews.apache.org/r/57474/diff/5/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 57474: Added test for authorization of
hierarchical roles.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57474/#review173164
-----------------------------------------------------------
Fix it, then Ship it!
LGTM. I will fix the tiny typo when I commit.
src/tests/authorization_tests.cpp
Line 1493 (original), 1493 (patched)
<https://reviews.apache.org/r/57474/#comment246237>
s/crete/create/
- Adam B
On April 18, 2017, 6:28 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57474/
> -----------------------------------------------------------
>
> (Updated April 18, 2017, 6:28 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Bannier.
>
>
> Bugs: MESOS-7026
> https://issues.apache.org/jira/browse/MESOS-7026
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds tests for each of the actions which support hierarchical roles.
>
>
> Diffs
> -----
>
> src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d
>
>
> Diff: https://reviews.apache.org/r/57474/diff/5/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 57474: Added test for authorization of
hierarchical roles.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57474/
-----------------------------------------------------------
(Updated April 18, 2017, 3:28 p.m.)
Review request for mesos, Adam B and Benjamin Bannier.
Bugs: MESOS-7026
https://issues.apache.org/jira/browse/MESOS-7026
Repository: mesos
Description
-------
Adds tests for each of the actions which support hierarchical roles.
Diffs (updated)
-----
src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d
Diff: https://reviews.apache.org/r/57474/diff/5/
Changes: https://reviews.apache.org/r/57474/diff/4-5/
Testing
-------
`make check`
Thanks,
Alexander Rojas
Re: Review Request 57474: Added test for authorization of
hierarchical roles.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57474/
-----------------------------------------------------------
(Updated April 11, 2017, 1:47 a.m.)
Review request for mesos, Adam B and Benjamin Bannier.
Bugs: MESOS-7026
https://issues.apache.org/jira/browse/MESOS-7026
Repository: mesos
Description
-------
Adds tests for each of the actions which support hierarchical roles.
Diffs
-----
src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d
Diff: https://reviews.apache.org/r/57474/diff/4/
Testing
-------
`make check`
Thanks,
Alexander Rojas
Re: Review Request 57474: Added test for authorization of
hierarchical roles.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57474/#review170963
-----------------------------------------------------------
Bad review!
Reviews applied: [57474, 57473, 57166, 56805, 57165, 57164]
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos Reviewbot
On April 4, 2017, 9:22 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57474/
> -----------------------------------------------------------
>
> (Updated April 4, 2017, 9:22 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Bannier.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds tests for each of the actions which support hierarchical roles.
>
>
> Diffs
> -----
>
> src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d
>
>
> Diff: https://reviews.apache.org/r/57474/diff/4/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Alexander Rojas
>
>