----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47399/#review133623 -----------------------------------------------------------
Fix it, then Ship it! I'll fix the last issues and commit shortly. include/mesos/authorizer/acls.proto (line 134) <https://reviews.apache.org/r/47399/#comment198338> s/operator/Operator include/mesos/authorizer/acls.proto (lines 141 - 144) <https://reviews.apache.org/r/47399/#comment198339> - Blank line - Wrap type into backticks - Comment fits two line include/mesos/authorizer/acls.proto (lines 153 - 156) <https://reviews.apache.org/r/47399/#comment198340> Ditto include/mesos/authorizer/acls.proto (line 182) <https://reviews.apache.org/r/47399/#comment198341> Rebase artifact? include/mesos/authorizer/acls.proto (line 220) <https://reviews.apache.org/r/47399/#comment198343> s/on/with include/mesos/authorizer/authorizer.proto (lines 61 - 63) <https://reviews.apache.org/r/47399/#comment198344> ``` // TODO(zhitao): Remove the following two actions at the end of // the deprecation cycle started with 0.29. They will be fully // replaced by `UPDATE_QUOTA_WITH_ROLE`. ``` src/authorizer/local/authorizer.cpp (line 198) <https://reviews.apache.org/r/47399/#comment198168> Comma after `QuotaHandler` src/authorizer/local/authorizer.cpp (lines 200 - 201) <https://reviews.apache.org/r/47399/#comment198351> // TODO(zhitao): Remove this special case at the end // of the deprecation cycle which started with 0.29. src/authorizer/local/authorizer.cpp (line 429) <https://reviews.apache.org/r/47399/#comment198350> s/valid/validationError src/authorizer/local/authorizer.cpp (line 462) <https://reviews.apache.org/r/47399/#comment198301> We usually use `Option<Error>` for validation routines. src/master/master.hpp (lines 1019 - 1021) <https://reviews.apache.org/r/47399/#comment198352> Move above after `authorizeGetQuota`. src/master/quota_handler.cpp (lines 341 - 344) <https://reviews.apache.org/r/47399/#comment198355> We can say it shorter: ``` list<Future<bool>> authorizeRequests = { authorizeSetQuota(principal, quotaInfo.role()), authorizeUpdateQuota(principal, quotaInfo.role())}; ``` src/master/quota_handler.cpp (line 348) <https://reviews.apache.org/r/47399/#comment198356> const &? src/master/quota_handler.cpp (line 468) <https://reviews.apache.org/r/47399/#comment198300> const &? src/tests/authorization_tests.cpp (line 1046) <https://reviews.apache.org/r/47399/#comment198182> I've discovered this is a general issue: MESOS-5405. I'm calling back my suggestion, let's remove this line for now, we need a proper fix instead of band aids. - Alexander Rukletsov On May 17, 2016, 9:11 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47399/ > ----------------------------------------------------------- > > (Updated May 17, 2016, 9:11 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 > b170330b236b83611d8477c0e45e520ef69aed9b > include/mesos/authorizer/authorizer.proto > 7d4aa32f42de538864508a0ba481d875917d9ab9 > src/authorizer/local/authorizer.hpp > d15d3a6c9f3d7c432e593cfe78cc48d672848d02 > src/authorizer/local/authorizer.cpp > bd47f4f02238e5052d9ab6caf338a2b51ddb752d > src/master/master.hpp 22a1538b8c2ca85cb8316aafe3f9a4476df833fc > src/master/quota_handler.cpp 86a2e175a3e1932961682c2a2d0cfe8210ee5fd0 > src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 > src/tests/master_quota_tests.cpp 9bfa6ca58d5a92b857e8f2ce5cb835ddf18b44e6 > > Diff: https://reviews.apache.org/r/47399/diff/ > > > Testing > ------- > > 1. Unit tests; > 2. Manually tested cases: authorized and forbidden under both deprecated > set_quotas/remove_quotas and new update_quotas, as well as the case that > specifying both triggers master crash. > > > Thanks, > > Zhitao Li > >
