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




sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Lines 99-101 (original), 99-101 (patched)
<https://reviews.apache.org/r/67131/#comment285332>

    This exception is not neeed anymore because we are preventing to happen 
from the asGroup() and asUser() methods. So the exception will never happen.



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Lines 103 (patched)
<https://reviews.apache.org/r/67131/#comment285334>

    Isn't HdfsAclEntity.asUser() clear enough to let the developers that you're 
creating a user entity? I'm just trying to keep a cleaner api for us, but it is 
not necessary to change it.



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Lines 275-279 (patched)
<https://reviews.apache.org/r/67131/#comment285335>

    will aclEntry be null? The static methods will never return null so we 
should be safe here.



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Lines 288 (patched)
<https://reviews.apache.org/r/67131/#comment285337>

    Can we have a comment here why FsAction.NONE is none for users and no for 
groups?



sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
Lines 290 (patched)
<https://reviews.apache.org/r/67131/#comment285336>

    aclEntry will never be null now that we use the static constructor, so we 
should be safe here.


- Sergio Pena


On May 15, 2018, 11:45 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67131/
> -----------------------------------------------------------
> 
> (Updated May 15, 2018, 11:45 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2174
>     https://issues.apache.org/jira/browse/SENTRY-2174
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SentryAuthorizationProvider should now additionally provide the ACL entries 
> with the permissions that users have along with the permissions for the 
> groups.
> 
> With the changes proposed in SENTRY-2173, PrivilegeInfo will not only have 
> role to permission mapping. it will also have user to privilege mapping 
> information.
> 
> SentryAuthorizationProvider should be using the new information added in 
> PrivilegeInfo to add ACL for users.
> 
> 
> Diffs
> -----
> 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java
>  a88d8e25ad745effe354aa6267252998b189a252 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryPermissions.java
>  dbce40538cf4721b601cf63b1da713f3d57fc981 
> 
> 
> Diff: https://reviews.apache.org/r/67131/diff/2/
> 
> 
> Testing
> -------
> 
> Added new unit tests and also made sure that all the existing tests pass.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Reply via email to