Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15206 )
Change subject: KUDU-2972 Add Ranger client ...................................................................... Patch Set 19: Code-Review+1 (5 comments) Overall looks good to me, thanks a lot for working on the patch Attila! http://gerrit.cloudera.org:8080/#/c/15206/19/src/kudu/ranger/ranger.proto File src/kudu/ranger/ranger.proto: http://gerrit.cloudera.org:8080/#/c/15206/19/src/kudu/ranger/ranger.proto@33 PS19, Line 33: // SELECT - select rows in a table : // INSERT - insert to a table : // UPDATE - update existing rows in a table : // DELETE - delete rows in a table : // ALTER - alter schema : // CREATE - creating a table : // DROP - drop a table nit: I think this can be removed now, and it is not quite accurate. http://gerrit.cloudera.org:8080/#/c/15206/16/src/kudu/ranger/ranger_client.cc File src/kudu/ranger/ranger_client.cc: http://gerrit.cloudera.org:8080/#/c/15206/16/src/kudu/ranger/ranger_client.cc@55 PS16, Line 55: atus RangerClient::Star > this will only call init, ranger authz provider sets up the server, do you A little surprised to see it is set in ranger authz provider as Ranger client should be able to successfully connect to Ranger subprocess on its own. However it is fine for now as mini Ranger is not in place. http://gerrit.cloudera.org:8080/#/c/15206/16/src/kudu/ranger/ranger_client.cc@67 PS16, Line 67: RETURN_NOT_OK(server_->Execute(&sreq, > that was the original approach, but it got optimized out and it segfaulted Ack, LGTM. http://gerrit.cloudera.org:8080/#/c/15206/16/src/kudu/ranger/ranger_client.cc@79 PS16, Line 79: : auto s = ParseRangerTableIdentifier(table_name, &db, &tbl); : if (PREDICT_FALSE(!s.ok())) { > Even with Sentry only conflicting tables had to be renamed. Tables with nam Ack http://gerrit.cloudera.org:8080/#/c/15206/19/src/kudu/ranger/ranger_client.cc File src/kudu/ranger/ranger_client.cc: http://gerrit.cloudera.org:8080/#/c/15206/19/src/kudu/ranger/ranger_client.cc@82 PS19, Line 82: Denying action on table with invalid name Maybe also advise the user to update the table name via table rename tool? -- To view, visit http://gerrit.cloudera.org:8080/15206 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2e1ec19ed3aeb4d82ad38fe1fb655f57021c1a4 Gerrit-Change-Number: 15206 Gerrit-PatchSet: 19 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 03 Mar 2020 19:15:43 +0000 Gerrit-HasComments: Yes
