Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11250 )
Change subject: IMPALA-7343: Update SentryProxy to use Sentry bulk API ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/11250/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11250/4//COMMIT_MSG@10 PS4, Line 10: ine > I guess that this "not" is not needed here. Done http://gerrit.cloudera.org:8080/#/c/11250/4//COMMIT_MSG@12 PS4, Line 12: rol > double "all" Done http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java File fe/src/main/java/org/apache/impala/util/SentryPolicyService.java: http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java@457 PS4, Line 457: s(requestingUser.getSh > optional: I would prefer to merge this somehow with listAllRolesPrivileges. Good idea. Done. http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java File fe/src/main/java/org/apache/impala/util/SentryProxy.java: http://gerrit.cloudera.org:8080/#/c/11250/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@243 PS4, Line 243: existingPrincipalPriv.setCatalogVersion( : catalog_.incrementAndGetCatalogVersion()); : } : continue; : } : if (principal.getPrincipalType() == TPrincipalType.ROLE) { : catalog_.addRolePrivilege(principal.getName(), thriftPriv); : } else { : catalog_.addUserPrivilege(principal.getName(), thriftPriv); : } : } : : // Remove the privileges that no longer exist. : for (String privilegeName: privilegesToRemove) { : TPrivilege privilege = new TPrivilege(); : privilege.setPrivilege_name(privilegeName); : if (principal.getPrincipalType() == TPrincipalType.ROLE) { : catalog_.removeRolePrivilege(principal.getName(), privilege); : } else { : catalog_.removeUserPrivilege(principal.getName(), privilege); : } : } : } : } : : /** : * Chec > optional: this seems very similar to its pair in refreshRolePrivileges - it Good idea. Done. -- To view, visit http://gerrit.cloudera.org:8080/11250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3ab215f2a5c5111bf1d25eec7fac90506d2f6304 Gerrit-Change-Number: 11250 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Adam Holley <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Fri, 17 Aug 2018 16:56:13 +0000 Gerrit-HasComments: Yes
