> On Feb. 22, 2016, 1:45 a.m., Dapeng Sun wrote:
> > sentry-policy/pom.xml, line 34
> > <https://reviews.apache.org/r/43704/diff/2/?file=1253500#file1253500line34>
> >
> >     do we need a new module? can we put them into common?

For sentry-policy-common, it is a dependency of sentry-provider-common, and 
CommonPolicy should depend on sentry-provider-common. If using the 
sentry-policy-common, there will be a circle denpendency problem.
In the other hand, both AuthorizationProvider and ProviderBackend in 
sentry-provider-common is not reasonable. They should be seperated.


> On Feb. 22, 2016, 1:45 a.m., Dapeng Sun wrote:
> > sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DatabaseRequiredInPrivilege.java,
> >  line 50
> > <https://reviews.apache.org/r/43704/diff/2/?file=1253502#file1253502line50>
> >
> >     It seems the property is related to database model, why we put it to 
> > PolicyConstant?

You're right, and this updated shouldn't be in this patch.


> On Feb. 22, 2016, 1:45 a.m., Dapeng Sun wrote:
> > sentry-policy/sentry-policy-engine/pom.xml, line 37
> > <https://reviews.apache.org/r/43704/diff/2/?file=1253503#file1253503line37>
> >
> >     Why we need hadoop common and minicluster dependencies?

Thanks for catch that, will update.


- Colin


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


On Feb. 18, 2016, 1:37 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43704/
> -----------------------------------------------------------
> 
> (Updated Feb. 18, 2016, 1:37 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Create CommonPolicy for external component
> 
> 
> Diffs
> -----
> 
>   sentry-policy/pom.xml ef938a6 
>   
> sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PolicyConstants.java
>  8e4d465 
>   
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DatabaseRequiredInPrivilege.java
>  e89aa16 
>   sentry-policy/sentry-policy-engine/pom.xml PRE-CREATION 
>   
> sentry-policy/sentry-policy-engine/src/main/java/org/apache/sentry/policy/engine/common/CommonPolicyEngine.java
>  PRE-CREATION 
>   
> sentry-policy/sentry-policy-engine/src/main/java/org/apache/sentry/policy/engine/common/CommonPrivilegeFactory.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDBConfiguration.java
>  985f969 
> 
> Diff: https://reviews.apache.org/r/43704/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Reply via email to