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


Fix it, then Ship it!




Thank Colin, I have only one comment.


sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PrivilegeConstants.java
 (line 22)
<https://reviews.apache.org/r/42935/#comment177972>

    I think PolicyConstants may be better.


- Dapeng Sun


On 一月 29, 2016, 9:39 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42935/
> -----------------------------------------------------------
> 
> (Updated 一月 29, 2016, 9:39 a.m.)
> 
> 
> Review request for sentry and Dapeng Sun.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> All constants in ProviderConstants are for privilege string, it's better to 
> move this class to sentry-policy-common and renamed to PrivilegeConstants.
> 
> 
> Diffs
> -----
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryIniPolicyFileFormatter.java
>  79164da 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestSentryIniPolicyFileFormatter.java
>  655417b 
>   sentry-policy/sentry-policy-common/pom.xml 68ada23 
>   
> sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/KeyValue.java
>  PRE-CREATION 
>   
> sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PrivilegeConstants.java
>  PRE-CREATION 
>   
> sentry-policy/sentry-policy-common/src/test/java/org/apache/sentry/policy/common/TestKeyValue.java
>  PRE-CREATION 
>   
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/AbstractDBPrivilegeValidator.java
>  e940fc3 
>   
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBModelAuthorizables.java
>  f07eb11 
>   
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBWildcardPrivilege.java
>  dfc2872 
>   
> sentry-policy/sentry-policy-db/src/test/java/org/apache/sentry/policy/db/TestDBWildcardPrivilege.java
>  bf5cec5 
>   
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/AbstractIndexerPrivilegeValidator.java
>  8520d1a 
>   
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerModelAuthorizables.java
>  e561962 
>   
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java
>  ab6b27f 
>   
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerWildcardPrivilege.java
>  5348f95 
>   
> sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/AbstractSearchPrivilegeValidator.java
>  781e722 
>   
> sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/SearchModelAuthorizables.java
>  dcf17a2 
>   
> sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/SearchWildcardPrivilege.java
>  c522412 
>   
> sentry-policy/sentry-policy-search/src/test/java/org/apache/sentry/policy/search/TestSearchWildcardPrivilege.java
>  125f358 
>   
> sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/ServerNameRequiredMatch.java
>  bbbcedd 
>   
> sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/SqoopModelAuthorizables.java
>  223fb55 
>   
> sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/SqoopWildcardPrivilege.java
>  139cf7f 
>   
> sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopWildcardPrivilege.java
>  1f03f05 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/KeyValue.java
>  984fe46 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderConstants.java
>  c6f7e2c 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  fef4bd9 
>   
> sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestKeyValue.java
>  1ae4c0c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeObject.java
>  c6e4aa6 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java
>  45f9ce4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SolrTSentryPrivilegeConvertor.java
>  f6a4f15 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java
>  56bbb8f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  530bdc7 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/CommandUtil.java
>  ffccec2 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/command/hive/ListPrivilegesCmd.java
>  98fae95 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
>  46798a0 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java
>  9d0a2d6 
>   
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/LocalGroupMappingService.java
>  1c12f11 
>   
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java
>  3a648a5 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  614856f 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPolicyImportExport.java
>  2482eb4 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/SentryPolicyProviderForDb.java
>  d0994b6 
> 
> Diff: https://reviews.apache.org/r/42935/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Reply via email to