----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47399/#review133537 -----------------------------------------------------------
Could you please also manually test the upgarde path for an operator? src/tests/authorization_tests.cpp (lines 977 - 978) <https://reviews.apache.org/r/47399/#comment198015> Blank line. src/tests/authorization_tests.cpp (line 1024) <https://reviews.apache.org/r/47399/#comment198004> s/request/requests src/tests/authorization_tests.cpp (lines 1042 - 1044) <https://reviews.apache.org/r/47399/#comment198013> If I remember correctly, this renders the message in a non-initialized state. It looks like it does not matter here, but I'd rather stay punctual. I think calling `request.mutable_subject();` is enough. src/tests/authorization_tests.cpp (lines 1060 - 1064) <https://reviews.apache.org/r/47399/#comment198018> Thanks for this test! src/tests/authorization_tests.cpp (lines 1074 - 1075) <https://reviews.apache.org/r/47399/#comment198014> One blank line is enough. src/tests/authorization_tests.cpp (line 1083) <https://reviews.apache.org/r/47399/#comment198019> s/get error/error out src/tests/authorization_tests.cpp (lines 1097 - 1098) <https://reviews.apache.org/r/47399/#comment198016> Ditto. src/tests/authorization_tests.cpp (line 1106) <https://reviews.apache.org/r/47399/#comment198020> s/get error/error out src/tests/master_quota_tests.cpp (line 1151) <https://reviews.apache.org/r/47399/#comment198031> set_quotas? src/tests/master_quota_tests.cpp (line 1158) <https://reviews.apache.org/r/47399/#comment198026> Let's call them "modify quotas and read status". src/tests/master_quota_tests.cpp (lines 1203 - 1204) <https://reviews.apache.org/r/47399/#comment198028> I find it unfortunate that we are inconsistent about wrapping for these blobs, even througout this file. I've grepped the codebase and it looks like we prefer ``` AWAIT_EXPECT_RESPONSE_STATUS_EQ(Forbidden().status, response) << response.get().body; ``` Could you please adjust it in your patch? Also, feel free to file a follow-up cleaning patch. src/tests/master_quota_tests.cpp (line 1207) <https://reviews.apache.org/r/47399/#comment198027> "Update" is misleading in the context. Technically we don't support updates yet; here you're setting quota, since you're POSTing. src/tests/master_quota_tests.cpp (line 1214) <https://reviews.apache.org/r/47399/#comment198023> Ups! src/tests/master_quota_tests.cpp (line 1217) <https://reviews.apache.org/r/47399/#comment198025> And here. src/tests/master_quota_tests.cpp (line 1228) <https://reviews.apache.org/r/47399/#comment198024> Ups I did it again! - 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 > >
