Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests ...................................................................... Patch Set 7: (1 comment) Hi Kurt and Quanlong, I left some additional thoughts on how we should pass an instance of group mapper to RangerAuthorizationChecker.java. Will try to implement the proposed approach in the next iteration since I think the current approach is too hacky and less flexible. Thanks! http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@314 PS7, Line 314: if (RuntimeEnv.INSTANCE.isTestEnv()) { After some thoughts, I feel like my approach here is a bit too hacky. A better way is to change that UserGroupInformation to a field of RangerAuthorizationChecker.java. Specifically, I think the following revision would be a better solution. 1. Add an additional field of UserGroupInformation in the class RangerAuthorizationChecker that should be initialized when instantiating a RangerAuthorizationChecker. 2. Add a new constructor of RangerAuthorizationFactory() that takes in an additional argument that allows a user to provide an instance of UserGroupInformation (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java#L51). 3. Add a new constructor of RangerAuthorizationChecker() that takes in additional argument of UserGroupInformation so that the checker can initialize its own instance of UserGroupInformation passed from its caller, i.e., RangerAuthorizationFactory() in this case (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L72). I will try to implement the proposed idea above in the next iteration. Thanks! -- To view, visit http://gerrit.cloudera.org:8080/14798 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2 Gerrit-Change-Number: 14798 Gerrit-PatchSet: 7 Gerrit-Owner: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Thu, 12 Dec 2019 17:22:29 +0000 Gerrit-HasComments: Yes
