-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34086/#review127748
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
 (line 506)
<https://reviews.apache.org/r/34086/#comment191136>

    validateClientVersion(request.getProtocol_version()); is needed



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
 (line 543)
<https://reviews.apache.org/r/34086/#comment191137>

    validateClientVersion(request.getProtocol_version());
    
    is needed.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
 (line 680)
<https://reviews.apache.org/r/34086/#comment191134>

    I saw the similar method for group, there is client version check as 
following:
    
    validateClientVersion(request.getProtocol_version());
    
    Is this necessary for this method?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
 (line 694)
<https://reviews.apache.org/r/34086/#comment191135>

    In concept, we may don't need to list the roles for the groups of the user.
    
    The command is to list the roles directly assigned to the user.


- Jerry Chen


On April 8, 2016, 6:40 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34086/
> -----------------------------------------------------------
> 
> (Updated April 8, 2016, 6:40 a.m.)
> 
> 
> Review request for sentry, Dapeng Sun and Jerry Chen.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Update SentryPolicyStoreProcessor for grant user to role
> 
> 
> Diffs
> -----
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
>  de50adb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
>  edc5661 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
>  8881d82 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerForHaWithoutKerberos.java
>  ac4df77 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java
>  0792eb6 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
>  07c7f7a 
> 
> Diff: https://reviews.apache.org/r/34086/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Reply via email to