Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13525 )
Change subject: IMPALA-8551: Bump CDP_BUILD_NUMBER to 1153860 ...................................................................... Patch Set 5: (2 comments) Looks pretty good modulo some nits. One thing, though: would it be possible to split this into one commit to bump the CDP_BUILD_NUMBER and one to change the error messages? I'm afraid when we look through lists of commits to backport across branches, we'll see this and think "oh it's just a CDP version bump, we can skip this one and pick up a later one" whereas in fact it also contains a functional change. http://gerrit.cloudera.org:8080/#/c/13525/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/13525/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@173 PS5, Line 173: } catch (Exception e) { pre-existing issue, but: is it possible to add a DEBUG level log message here which includes the stack trace of the exception? I'm afraid at some point we'll trigger an NPE or something within the Ranger client code, and the excepption will get swallowed, and all we'll see is "null" in the error message. (same below) http://gerrit.cloudera.org:8080/#/c/13525/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@174 PS5, Line 174: throw new InternalException("Error granting a privilege to Ranger. " + nit: can we reword this to "in Ranger" instead of "to Ranger" -- the privilege is being granted to some target user, not to the ranger service itself. -- To view, visit http://gerrit.cloudera.org:8080/13525 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8995f5dc88b211cd3af415713802cfeac44fe576 Gerrit-Change-Number: 13525 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Mon, 10 Jun 2019 15:50:38 +0000 Gerrit-HasComments: Yes
