Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-2660: Respect auth_to_local configs from hdfs configs ......................................................................
Patch Set 8: -Code-Review (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 Discussed with Alex over chat. I tried this already but this doesn't work as expected, because User class is used across daemons and not just impalad and they might not have access to fe-support. 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 wrap i tried doing this but Java doesn't allow overriding methods to throw exceptions that are broader than the super class' method. In this case super class method throws IOE but we can't wrap it and throw InternalException because it extends "Exception" class directly. http://stackoverflow.com/questions/5875414/why-cant-overriding-methods-throw-exceptions-broader-than-the-overriden-method 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 Done. Nice Idea, looks much better now. -- 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
