sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Prasad Mujumdar" <pras...@cloudera.com>
Subject Re: Review Request 31657: SENTRY-644: Sentry Sqoop binding framework for role-based authorization
Date Sat, 21 Mar 2015 02:16:04 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message