Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15074 )
Change subject: [java] KUDU-2972: add Kudu Ranger plugin ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/15074/3/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java File java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java: http://gerrit.cloudera.org:8080/#/c/15074/3/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@30 PS3, Line 30: import org.apache.ranger.plugin.service.RangerBasePlugin; > Should we also use mockito to test this class by generating some expected R Done http://gerrit.cloudera.org:8080/#/c/15074/3/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@66 PS3, Line 66: rn the authorizati > If authorize() is to return false, maybe we should also debug log the Range Done http://gerrit.cloudera.org:8080/#/c/15074/3/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@67 PS3, Line 67: */ > Probably meant to do something with this? Do we need to retry if this is fa Sorry from what I am aware that we don't care about whether the result is determined, as we consider empty result to be denied by default. And for retry, I think it is not necessary in this case, as if the ranger server is not available the plugin will authorize based on the cache if I understand it correctly. I will remove this. http://gerrit.cloudera.org:8080/#/c/15074/3/java/kudu-ranger/src/test/java/org/apache/kudu/ranger/TestRangerSubprocess.java File java/kudu-ranger/src/test/java/org/apache/kudu/ranger/TestRangerSubprocess.java: http://gerrit.cloudera.org:8080/#/c/15074/3/java/kudu-ranger/src/test/java/org/apache/kudu/ranger/TestRangerSubprocess.java@26 PS3, Line 26: > nit: reorder? Done http://gerrit.cloudera.org:8080/#/c/15074/3/java/kudu-ranger/src/test/java/org/apache/kudu/ranger/TestRangerSubprocess.java@69 PS3, Line 69: break; > Maybe consider a test that sends multiple requests before serializing? And Done -- To view, visit http://gerrit.cloudera.org:8080/15074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0c995ac1a48ebf57667231cd3a82d3794f6ddf8d Gerrit-Change-Number: 15074 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao <[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-Comment-Date: Wed, 12 Feb 2020 00:06:42 +0000 Gerrit-HasComments: Yes
