----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47399/#review133421 -----------------------------------------------------------
I've committed MESOS-5336, I believe you have to rebase this one. include/mesos/authorizer/acls.proto (line 124) <https://reviews.apache.org/r/47399/#comment197838> You don't have to put my name here : ). Feel free to put yours, if you like; it is your patch. Here and everywhere. include/mesos/authorizer/acls.proto (lines 165 - 182) <https://reviews.apache.org/r/47399/#comment197839> Though it does not influence the thrust, it would be great if we maintain the same ordering everywhere. I'd vote for keeping them after dynamic volumes, but feel free to put them at the end as well. I care more about consistency. src/authorizer/local/authorizer.cpp (lines 69 - 70) <https://reviews.apache.org/r/47399/#comment197844> No period at the end of log messages. I see this is in sync with other log messages in this file, but we tend to put the space in the beginning of the next line, so it's easier to spot when they are missing. You don't need to change it here, so we stay locally consistent. src/authorizer/local/authorizer.cpp (lines 195 - 198) <https://reviews.apache.org/r/47399/#comment197994> How about ``` // Deprecation case: If `update_quotas` is empty but // `set_quotas` or `remove_quotas` are not, "skip" // authorization here under assumption that the caller, // i.e., `QuotaHandler` checks `set_quotas`/`remove_quotas`. ``` src/authorizer/local/authorizer.cpp (line 462) <https://reviews.apache.org/r/47399/#comment197993> I like the idea of validating ACLs on costruction. I'm thinking what else should we validate here. src/authorizer/local/authorizer.cpp (lines 468 - 469) <https://reviews.apache.org/r/47399/#comment197992> Blank line. src/master/quota_handler.cpp (line 557) <https://reviews.apache.org/r/47399/#comment197995> No period, please. Also, wrap `role` in single quotes and sqush onto the previous line. src/master/quota_handler.cpp (lines 566 - 567) <https://reviews.apache.org/r/47399/#comment197997> Blank line, please. - Alexander Rukletsov On May 15, 2016, 4:30 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47399/ > ----------------------------------------------------------- > > (Updated May 15, 2016, 4:30 p.m.) > > > Review request for mesos, Adam B and Alexander Rukletsov. > > > Bugs: MESOS-5155 > https://issues.apache.org/jira/browse/MESOS-5155 > > > Repository: mesos > > > Description > ------- > > New update_quotas ACL for both set and remove cases. > > > Diffs > ----- > > include/mesos/authorizer/acls.proto > 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d > include/mesos/authorizer/authorizer.proto > 32492a59ad95df3bb673ec42321518f86c11af59 > src/authorizer/local/authorizer.hpp > d15d3a6c9f3d7c432e593cfe78cc48d672848d02 > src/authorizer/local/authorizer.cpp > e95435327bb3b6f447e814b8657bce8084535346 > src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 > src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c > src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 > src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 > > Diff: https://reviews.apache.org/r/47399/diff/ > > > Testing > ------- > > Unit tests. > > > Thanks, > > Zhitao Li > >
