Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12334 )
Change subject: IMPALA-8150: Fix buggy AuditingTest::TestAccessEventsOnAuthFailure ...................................................................... Patch Set 4: (2 comments) Thanks for cleaning this up, and for fixing the "polarity" of the assertEquals statements. A few minor comments. http://gerrit.cloudera.org:8080/#/c/12334/4/fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java File fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java: http://gerrit.cloudera.org:8080/#/c/12334/4/fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java@83 PS4, Line 83: if (sentryConfig_.getConfigFile() != null) { You've found that something prevents the file name from ever being ""? Or, that we want to fail the load in this case? http://gerrit.cloudera.org:8080/#/c/12334/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java: http://gerrit.cloudera.org:8080/#/c/12334/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@962 PS4, Line 962: "Authorization is enabled but the server name is null or empty. Set the " + Error message says null or empty, but the code was changed to just check null... -- To view, visit http://gerrit.cloudera.org:8080/12334 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I712697e6d5a3e171b259a781bd07de9871a29b95 Gerrit-Change-Number: 12334 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Paul Rogers <prog...@cloudera.com> Gerrit-Comment-Date: Sat, 02 Feb 2019 03:19:23 +0000 Gerrit-HasComments: Yes