Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12637 )
Change subject: IMPALA-7916: Remove support for authorization policy file flag ...................................................................... Patch Set 4: (5 comments) Thanks! Overall it looks good now, just couple nits. http://gerrit.cloudera.org:8080/#/c/12637/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/12637/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@51 PS4, Line 51: import org.apache.impala.thrift.*; nit: don't use wildcard imports http://gerrit.cloudera.org:8080/#/c/12637/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@54 PS4, Line 54: import org.junit.*; same thing here http://gerrit.cloudera.org:8080/#/c/12637/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@68 PS4, Line 68: private static final SentryAuthorizationConfig authzConfig = : SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1", : System.getenv("IMPALA_HOME") + "/fe/src/test/resources/sentry-site.xml"); : private static final ImpaladTestCatalog authzCatalog_ = : new ImpaladTestCatalog(authzConfig); : private static final Frontend authzFe_ = new Frontend(authzConfig, authzCatalog_); static final variables should use upper case names http://gerrit.cloudera.org:8080/#/c/12637/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@78 PS4, Line 78: analysisContext_ can we rename this to authzCtx to differentiate between normal ctx vs authz ctx? http://gerrit.cloudera.org:8080/#/c/12637/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@80 PS4, Line 80: setupImpalaCatalog(authzCatalog_); : } catch(ImpalaException e) { : throw new RuntimeException(e); : } We can just declare the ImpalaException in the constructor. -- To view, visit http://gerrit.cloudera.org:8080/12637 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic2a52c2d5d35f58fbff8c088fb0bf30169625ebd Gerrit-Change-Number: 12637 Gerrit-PatchSet: 4 Gerrit-Owner: Austin Nobis <[email protected]> Gerrit-Reviewer: Austin Nobis <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Fri, 01 Mar 2019 17:37:13 +0000 Gerrit-HasComments: Yes
