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




docs/authorization.md (line 8)
<https://reviews.apache.org/r/46501/#comment195038>

    First sentence: I would instead say something like, "In Mesos, the 
authorization subsystem allows the operator to configure the actions that 
certain principals are allowed to perform. For example, the operator can use 
authorization to ensure that principal `foo` can only register frameworks in 
role `bar`, and no other principals can register frameworks in any role."
    
    I'd probably add a newline before "A reference"
    
    You might want to clarify that (a) ACLs are a property of the local 
authorizer only; other authorizers might use different concepts for deciding 
whether a principal is allowed to perform an action, (b) the remainder of the 
document (?) is specific to the local authorizer; other authorizors might 
behave differently.



docs/authorization.md (line 12)
<https://reviews.apache.org/r/46501/#comment195036>

    "through a"
    
    "configured with"



docs/authorization.md (line 27)
<https://reviews.apache.org/r/46501/#comment195042>

    Did you remove the "Role vs. Principal" discussion deliberately? If so, why?



docs/authorization.md 
<https://reviews.apache.org/r/46501/#comment195045>

    The previous text had an example for every ACL action. I think giving an 
example for every ACL action helps the reader understand how to use ACLs more 
concretely.



docs/authorization.md (line 64)
<https://reviews.apache.org/r/46501/#comment195039>

    "The order in which the rules are defined is important."
    
    "the acls" => "the ACLs"



docs/authorization.md (line 82)
<https://reviews.apache.org/r/46501/#comment195040>

    "ACLs"



docs/authorization.md (line 104)
<https://reviews.apache.org/r/46501/#comment195046>

    You might consider presenting this information using a Markdown table, 
rather than a list where every element of the list has the same format.



docs/authorization.md (line 105)
<https://reviews.apache.org/r/46501/#comment195047>

    "ACLs entry" is imprecise; I would say it is the thing you're describing is 
the "action", and a single ACL consists of the triple (subject, action, object).



docs/authorization.md (line 114)
<https://reviews.apache.org/r/46501/#comment195043>

    For consistency, this needs a newline (`<br/>`).



docs/authorization.md (line 160)
<https://reviews.apache.org/r/46501/#comment195049>

    The actual signature is `process::Future<bool> authorized(const 
authorization::Request& request)`, right?
    
    Also, why are we using three back-ticks to denote inline code? AFAIK we 
should use a single back-tick for that; three back-ticks are for code blocks.
    
    I would avoid saying "authorized() should return `false`..." -- 
authorized() returns a future. I'd say something like "authorized() returns a 
future that indicates the result of the (asynchronous) authorization operation. 
If the future is set to `true`, the request was authorized successfully; if it 
was set to `false`, ..."


- Neil Conway


On April 28, 2016, 9:56 a.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46501/
> -----------------------------------------------------------
> 
> (Updated April 28, 2016, 9:56 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-4785
>     https://issues.apache.org/jira/browse/MESOS-4785
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The API of the authorization has been changing constantly over the
> last few versions. This patch attempts to update the documentation to
> the those changes into account.
> 
> 
> Diffs
> -----
> 
>   docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
> 
> Diff: https://reviews.apache.org/r/46501/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Reply via email to