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


Fix it, then Ship it!




Looks great - one nit on the proto message member naming.


docs/authorization.md
Lines 858 (patched)
<https://reviews.apache.org/r/58964/#comment251283>

    Not yours, but it seems we are missing a bunch here -- we should get them 
complete - mind doing this here or in a follow-up?
    
    ```
      optional quota.QuotaInfo quota_info = 6;
      optional WeightInfo weight_info = 7;
      optional Resource resource = 8;
      optional CommandInfo command_info = 9;
      optional ContainerID container_id = 10;
    ```



include/mesos/authorizer/acls.proto
Lines 472-476 (patched)
<https://reviews.apache.org/r/58964/#comment251284>

    We are using repeated here, hence we are allowing for multiple elements and 
we do always use plural for the naming of such proto message arrays.
    
    In this case, maybe we regard each of the members as an ACL; add "es" for a 
shortened way of saying ACLs.
    
    ```
      repeated ACL.UpdateMaintenanceSchedule update_maintenance_schedules = 35;
      repeated ACL.GetMaintenanceSchedule get_maintenance_schedules = 36;
      repeated ACL.StartMaintenance start_maintenances = 37;
      repeated ACL.StopMaintenance stop_maintenances = 38;
      repeated ACL.GetMaintenanceStatus get_maintenance_statuses = 39;
    ```


- Till Toenshoff


On June 2, 2017, 12:32 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58964/
> -----------------------------------------------------------
> 
> (Updated June 2, 2017, 12:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7415
>     https://issues.apache.org/jira/browse/MESOS-7415
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Adds the actions `UPDATE_MAINTENANCE_SCHEDULE`,
> `GET_MAINTENANCE_SCHEDULE`, `START_MAINTENANCE`, `STOP_MAINTENANCE`
> and `GET_MAINTENANCE_STATUS` to the authorizer API as well as the
> necesary code to handle these new actions.
> 
> While the interface `mesos::Authorizer` takes an object with type
> `MachineID` to perform authorization; the default implementation of
> the interface `mesos::LocalAuthorizer` ignores the object choosing the
> semantics of allow maintenance on all nodes or none. This was done to
> extend the capacities of custom authorizers which may have special
> rules for authorization.
> 
> 
> Diffs
> -----
> 
>   docs/authorization.md d94f0f9d142e66118b89ecac28b9a4b21e88b6c8 
>   include/mesos/authorizer/acls.proto 
> 36b3ac231d8be9d40aad08940e870a69b0b72aa8 
>   include/mesos/authorizer/authorizer.hpp 
> 4a7376fb6ca2be0a513ad54f56eea3cf8cdd024d 
>   include/mesos/authorizer/authorizer.proto 
> c9184d151befa4cea9bdebb36a315c760e6424b2 
>   src/authorizer/local/authorizer.cpp 
> 1f2a9902e88705cf412af834c127c3afe6d3bef4 
>   src/tests/authorization_tests.cpp 6d85a5452d50390f96e0b11c2bac5c96722f6eac 
> 
> 
> Diff: https://reviews.apache.org/r/58964/diff/6/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Reply via email to