Alex Behm has posted comments on this change.

Change subject: IMPALA-2660: Respect auth_to_local configs from hdfs configs
......................................................................


Patch Set 8:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/2800/8/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:

Line 72:   jboolean auth_to_local = FLAGS_load_auth_to_local_rules && 
!FLAGS_principal.empty();
I'd prefer to use the existing mechanism to plumb BE configurations to the FE.

See:
TStartupOptions in Frontend.thrift

RuntimeEnv in the FE which calls FeSupport.GetStartupOptions() which is 
implemented in fe-support.cc


http://gerrit.cloudera.org:8080/#/c/2800/8/fe/src/main/java/com/cloudera/impala/authorization/AuthorizationChecker.java
File 
fe/src/main/java/com/cloudera/impala/authorization/AuthorizationChecker.java:

Line 114:     try {
can we simplify all these try catches by overriding getShortName() and wrapping 
it in a try super.getShortName() catch


http://gerrit.cloudera.org:8080/#/c/2800/8/fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java
File fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java:

Line 1308:       throws AnalysisException, AuthorizationException, 
InternalException {
throws ImpalaException seems simpler


-- 
To view, visit http://gerrit.cloudera.org:8080/2800
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I76485b83c14ba26f6fce66e5f83e8014667829e0
Gerrit-PatchSet: 8
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Juan Yu <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-HasComments: Yes

Reply via email to