Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8588 )
Change subject: IMPALA-4927: Impala should handle invalid input from Sentry ...................................................................... Patch Set 8: (1 comment) I'm not too familiar with the details here, but I noticed that TSentryGrantOption can be false as well as true (and unset). Could a role deny privileges? If so, does removing that role (because we couldn't load it for some reason) potentially increase the privileges of the person operating the query? In other words, when we fail, do we need to leave a marker to indicate that role X is poisonous and we should fail anything that needs it? http://gerrit.cloudera.org:8080/#/c/8588/5/fe/src/main/java/org/apache/impala/util/SentryProxy.java File fe/src/main/java/org/apache/impala/util/SentryProxy.java: http://gerrit.cloudera.org:8080/#/c/8588/5/fe/src/main/java/org/apache/impala/util/SentryProxy.java@146 PS5, Line 146: Exception e) { > Changed the code to handle all exceptions I'm not sure I agree with Bharath. We don't catch generic exceptions (like ConnectionException, timeouts, NPE, ClassNotFound, and so on) for things like HDFS operations. Sentry is supposed to have an API such that we know what exceptions it can and can't throw, and we should catch based on that. -- To view, visit http://gerrit.cloudera.org:8080/8588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b Gerrit-Change-Number: 8588 Gerrit-PatchSet: 8 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-Comment-Date: Tue, 21 Nov 2017 19:02:37 +0000 Gerrit-HasComments: Yes
