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




sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java
Lines 60 (patched)
<https://reviews.apache.org/r/66590/#comment282874>

    should we make this function thread-safe? It invoves two tables and they 
have to be consistent.



sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java
Lines 101 (patched)
<https://reviews.apache.org/r/66590/#comment282871>

    do we need to make this function thread-safe? Otherwise, those two tables 
may be in inconsistent state.



sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java
Lines 207 (patched)
<https://reviews.apache.org/r/66590/#comment282879>

    This function name is strange. It just returns the sentry object, not action



sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java
Lines 20 (patched)
<https://reviews.apache.org/r/66590/#comment282884>

    Can you add google doc?



sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java
Lines 31 (patched)
<https://reviews.apache.org/r/66590/#comment282885>

    can you add google doc for this class?



sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java
Lines 116 (patched)
<https://reviews.apache.org/r/66590/#comment282890>

    should each policy has an index? So you can print the index and it is easy 
to debug and correleate events.



sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java
Lines 119 (patched)
<https://reviews.apache.org/r/66590/#comment282891>

    This logic has some problem. will the policies have priorities? then they 
should be sorted before you check them and need to handle conflict.
    
    If you don't do that, what's the assumption the policies behave?



sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java
Lines 48 (patched)
<https://reviews.apache.org/r/66590/#comment282896>

    do you expect someone calls this function?
    
    It seems you should have a internal thread to periodically pull other 
places to get delta change.


- Na Li


On April 19, 2018, 9:09 p.m., Steve Moist wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66590/
> -----------------------------------------------------------
> 
> (Updated April 19, 2018, 9:09 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This is the inital draft of attribute based access control.
> 
> 
> Diffs
> -----
> 
>   pom.xml 16a3838a 
>   sentry-abac/example-definition.json PRE-CREATION 
>   sentry-abac/example-delta.json PRE-CREATION 
>   sentry-abac/notes.txt PRE-CREATION 
>   sentry-abac/pom.xml PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/Attribute.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMap.java 
> PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapAdapter.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/AttributeMapKeyException.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/GenericAttributeProvider.java
>  PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/SentryAttributeAuthorizer.java
>  PRE-CREATION 
>   sentry-abac/src/main/java/org/apache/sentry/abac/SentryObject.java 
> PRE-CREATION 
>   
> sentry-abac/src/main/java/org/apache/sentry/abac/StaticAttributeProvider.java 
> PRE-CREATION 
>   sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttribute.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestAttributeMap.java 
> PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestSentryAttributeAuthorizer.java
>  PRE-CREATION 
>   
> sentry-abac/src/test/java/org/apache/sentry/tests/abac/TestStaticProvider.java
>  PRE-CREATION 
>   sentry-abac/src/test/resources/abac.props PRE-CREATION 
>   sentry-binding/sentry-binding-hive/pom.xml ccfa9cfe 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryValidator.java
>  1ab5be35 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryHiveAuthorizerImpl.java
>  86ff0cc2 
> 
> 
> Diff: https://reviews.apache.org/r/66590/diff/5/
> 
> 
> Testing
> -------
> 
> full build,added unit tests, tested code on a cluster.
> 
> 
> Thanks,
> 
> Steve Moist
> 
>

Reply via email to