Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/15207 )
Change subject: [WIP] Ranger authorization provider ...................................................................... Patch Set 13: (9 comments) http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/catalog_manager.cc@754 PS12, Line 754: return Status::InvalidArgument(Substitute( : "column '$0' has write_default field set but no read_default", col->name())); : } : return Status::OK(); > Handle this as a GROUP_FLAG_VALIDATOR instead. Done http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h File src/kudu/master/ranger_authz_provider.h: http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h@38 PS12, Line 38: // An implementation of AuthzProvider that connects to Ranger and translates > Provide a high level class doc. Done http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h@40 PS12, Line 40: // the received response > Can this be omitted? Don't you get a public constructor by default? I think I need it if I want to provide a constructor definition. http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h@48 PS12, Line 48: : Status ResetCache() ove > You can implement this function inline and remove the docs since it'll be o Done http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc File src/kudu/master/ranger_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@40 PS12, Line 40: "Ranger authorization. sentry_service_rpc_addresses must not be " > We should update the equivalent flag in Sentry to indicate this incompatibi Done http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@57 PS12, Line 57: vector<string>({"ranger" > Don't need std prefixes. Done http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@65 PS12, Line 65: Status RangerAuthzProvider::AuthorizeCreateTable(const string& table_name, : > Can inline in the header. Done http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@95 PS12, Line 95: RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::ALL, old_table)); : return client_.AuthorizeAction(user, ActionPB::CREATE, new_table); : } : : Status RangerAuthzProvider::AuthorizeGetTableMetadata(const string& table_name, : const string > Could you write a comment similar to what's in SentryAuthzProvider to expla Done http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@128 PS12, Line 128: > Why not METADATA? Please add a comment. Done -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 02 Mar 2020 18:32:12 +0000 Gerrit-HasComments: Yes
