Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16072 )
Change subject: KUDU-3090 Add ownership privileges ...................................................................... Patch Set 6: (8 comments) http://gerrit.cloudera.org:8080/#/c/16072/4//COMMIT_MSG Commit Message: PS4: > This should be further split into two parts, because it attempts to do two Done http://gerrit.cloudera.org:8080/#/c/16072/4/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/16072/4/src/kudu/master/catalog_manager.cc@2954 PS4, Line 2954: size_t found = table_name.find(req->name_filt > Why can't you use an unordered_map? Done http://gerrit.cloudera.org:8080/#/c/16072/4/src/kudu/ranger/ranger_client-test.cc File src/kudu/ranger/ranger_client-test.cc: http://gerrit.cloudera.org:8080/#/c/16072/4/src/kudu/ranger/ranger_client-test.cc@247 PS4, Line 247: false); > Shouldn't these be booleans? Done http://gerrit.cloudera.org:8080/#/c/16072/4/src/kudu/ranger/ranger_client-test.cc@404 PS4, Line 404: ASSERT_OK(client_->AuthorizeAction("user", ActionPB::ALL, "db", "table", /*is_owner=*/false, > warning: argument name 'owner' in comment does not match parameter name 'is Done http://gerrit.cloudera.org:8080/#/c/16072/4/src/kudu/ranger/ranger_client-test.cc@424 PS4, Line 424: ASSERT_OK(client_->AuthorizeAction("user", ActionPB::ALL, "db", "table", /*is_owner=*/false, > warning: argument name 'owner' in comment does not match parameter name 'is Done http://gerrit.cloudera.org:8080/#/c/16072/4/src/kudu/ranger/ranger_client-test.cc@441 PS4, Line 441: ASSERT_OK(client_->AuthorizeAction("user", ActionPB::ALL, "db", "table", /*is_owner=*/false, > warning: argument name 'owner' in comment does not match parameter name 'is Done http://gerrit.cloudera.org:8080/#/c/16072/4/src/kudu/security/simple_acl.h File src/kudu/security/simple_acl.h: http://gerrit.cloudera.org:8080/#/c/16072/4/src/kudu/security/simple_acl.h@27 PS4, Line 27: trol list whic > nit: TableOwnerPair makes this sound like it should be a string, string pai Done http://gerrit.cloudera.org:8080/#/c/16072/4/src/kudu/security/simple_acl.h@29 PS4, Line 29: // This is basically just a wrapper around a set<string> with a bit of parsing logic and : // support for the '*' wildcard. : class SimpleAcl { : public: : > The fact that this doesn't take into account 'owner' seems wrong. Done -- To view, visit http://gerrit.cloudera.org:8080/16072 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id9c36b7d84863403d7d538cafc709d2aebd0b109 Gerrit-Change-Number: 16072 Gerrit-PatchSet: 6 Gerrit-Owner: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 26 Jun 2020 09:45:53 +0000 Gerrit-HasComments: Yes
