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



Thank Colin, I left some comments.


sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HiveActionFactory.java
 (line 24)
<https://reviews.apache.org/r/43818/#comment181675>

    Better to add some comments before the class



sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HiveActionFactory.java
 (line 37)
<https://reviews.apache.org/r/43818/#comment181662>

    Where is action **Some**, and could you add some comments for ALL and 
ALL_STAR



sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HiveActionFactory.java
 (line 57)
<https://reviews.apache.org/r/43818/#comment181678>

    return null?



sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HivePrivilegeModel.java
 (line 35)
<https://reviews.apache.org/r/43818/#comment181677>

    It seem the line is too long, can we use static import to reduce it?



sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HivePrivilegeModel.java
 (line 53)
<https://reviews.apache.org/r/43818/#comment181669>

    How about improve the thread safety here or "lazy initialization"?



sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerPrivilegeModel.java
 (line 34)
<https://reviews.apache.org/r/43818/#comment181671>

    I didn't find **IndexerActionFactory**



sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerPrivilegeModel.java
 (line 50)
<https://reviews.apache.org/r/43818/#comment181668>

    How about improve the thread safety here or "lazy initialization"?



sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchPrivilegeModel.java
 (line 34)
<https://reviews.apache.org/r/43818/#comment181672>

    where is SearchActionFactory



sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchPrivilegeModel.java
 (line 51)
<https://reviews.apache.org/r/43818/#comment181665>

    How about improve the thread safety here or "lazy initialization"?



sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopPrivilegeModel.java
 (line 34)
<https://reviews.apache.org/r/43818/#comment181674>

    where is SqoopActionFactory



sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopPrivilegeModel.java
 (line 53)
<https://reviews.apache.org/r/43818/#comment181666>

    How about improve the thread safety here or "lazy initialization"?


- Dapeng Sun


On 二月 22, 2016, 10:50 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43818/
> -----------------------------------------------------------
> 
> (Updated 二月 22, 2016, 10:50 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> For new external components, it should implement Model and 
> BitFieldActionFactory.
> Implement Model and BitFieldActionFactory for Hive, Sqoop, Solr, Index.
> 
> 
> Diffs
> -----
> 
>   
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HiveActionFactory.java
>  PRE-CREATION 
>   
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/HivePrivilegeModel.java
>  PRE-CREATION 
>   
> sentry-core/sentry-core-model-indexer/src/main/java/org/apache/sentry/core/model/indexer/IndexerPrivilegeModel.java
>  PRE-CREATION 
>   
> sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchPrivilegeModel.java
>  PRE-CREATION 
>   
> sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopPrivilegeModel.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43818/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Reply via email to