----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31657/#review77299 -----------------------------------------------------------
Looks fine. Some minor comments below. sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationHander.java <https://reviews.apache.org/r/31657/#comment125196> Does this need to be info ? I am not sure how often sqoop calls this. It might be too much noise in the log file. sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationHander.java <https://reviews.apache.org/r/31657/#comment125199> Should this be InstantiationException ? sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationHander.java <https://reviews.apache.org/r/31657/#comment125200> It would be great if SqoopConfig could directly indentify if strong authorization is enabled rather than Sentry figuring out that from sqoop config. Let's work with Sqoop comminuty to make this into an API in future. sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationValidator.java <https://reviews.apache.org/r/31657/#comment125201> " does not have privilege for action " ? sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java <https://reviews.apache.org/r/31657/#comment125243> Nit: formatting. sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java <https://reviews.apache.org/r/31657/#comment125240> Is there a reason we don't pass the full exception to SqoopException() ? - Prasad Mujumdar On March 18, 2015, 2:14 a.m., shen guoquan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31657/ > ----------------------------------------------------------- > > (Updated March 18, 2015, 2:14 a.m.) > > > Review request for sentry, Sqoop, Abraham Elmahrek, Xiaomeng Huang, Colin Ma, > Dapeng Sun, and Prasad Mujumdar. > > > Repository: sentry > > > Description > ------- > > Sentry Sqoop binding framework for role-based authorization > > > Diffs > ----- > > pom.xml 4c80916 > sentry-binding/pom.xml 7428aa5 > sentry-binding/sentry-binding-sqoop/pom.xml PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/PrincipalDesc.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/SentrySqoopError.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/NoopSentryAccessController.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAccessController.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationHander.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationValidator.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/conf/SqoopAuthConf.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/test/java/org/apache/sentry/sqoop/MockAuthenticationProvider.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/test/java/org/apache/sentry/sqoop/TestSentryAuthorizationHander.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/test/java/org/apache/sentry/sqoop/TestSqoopAuthConf.java > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/test/resources/no-configure-sentry-site.xml > PRE-CREATION > sentry-binding/sentry-binding-sqoop/src/test/resources/sentry-site.xml > PRE-CREATION > > sentry-binding/sentry-binding-sqoop/src/test/resources/test-authz-provider.ini > PRE-CREATION > sentry-dist/pom.xml f63b33b > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java > def3486 > > Diff: https://reviews.apache.org/r/31657/diff/ > > > Testing > ------- > > Ran unit tests > > > Thanks, > > shen guoquan > >
