You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Colin Ma <ju...@intel.com> on 2016/02/18 02:31:58 UTC
Review Request 43704: SENTRY-1042: Create CommonPolicy for external
component
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43704/
-----------------------------------------------------------
Review request for sentry.
Repository: sentry
Description
-------
Create CommonPolicy for external component
Diffs
-----
pom.xml 2f855fb
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
Re: Review Request 43704: SENTRY-1042: Create CommonPolicy for
external component
Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43704/#review120118
-----------------------------------------------------------
Ship it!
Ship It!
- Dapeng Sun
On 二月 22, 2016, 11:13 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43704/
> -----------------------------------------------------------
>
> (Updated 二月 22, 2016, 11:13 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Create CommonPolicy for external component
>
>
> Diffs
> -----
>
> sentry-policy/pom.xml ef938a6
> 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
>
> Diff: https://reviews.apache.org/r/43704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 43704: SENTRY-1042: Create CommonPolicy for
external component
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43704/
-----------------------------------------------------------
(Updated Feb. 22, 2016, 3:13 a.m.)
Review request for sentry.
Repository: sentry
Description
-------
Create CommonPolicy for external component
Diffs (updated)
-----
sentry-policy/pom.xml ef938a6
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
Diff: https://reviews.apache.org/r/43704/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 43704: SENTRY-1042: Create CommonPolicy for
external component
Posted by Dapeng Sun <da...@intel.com>.
> On 二月 22, 2016, 9: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?
>
> Colin Ma wrote:
> 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.
SGTM
- Dapeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43704/#review120085
-----------------------------------------------------------
On 二月 22, 2016, 11:13 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43704/
> -----------------------------------------------------------
>
> (Updated 二月 22, 2016, 11:13 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Create CommonPolicy for external component
>
>
> Diffs
> -----
>
> sentry-policy/pom.xml ef938a6
> 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
>
> Diff: https://reviews.apache.org/r/43704/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 43704: SENTRY-1042: Create CommonPolicy for
external component
Posted by Colin Ma <ju...@intel.com>.
> 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
>
>
Re: Review Request 43704: SENTRY-1042: Create CommonPolicy for
external component
Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43704/#review120085
-----------------------------------------------------------
Thank Colin, I left some comments.
sentry-policy/pom.xml (line 34)
<https://reviews.apache.org/r/43704/#comment181453>
do we need a new module? can we put them into common?
sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DatabaseRequiredInPrivilege.java (line 50)
<https://reviews.apache.org/r/43704/#comment181451>
It seems the property is related to database model, why we put it to PolicyConstant?
sentry-policy/sentry-policy-engine/pom.xml (line 37)
<https://reviews.apache.org/r/43704/#comment181452>
Why we need hadoop common and minicluster dependencies?
- Dapeng Sun
On 二月 18, 2016, 9:37 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43704/
> -----------------------------------------------------------
>
> (Updated 二月 18, 2016, 9: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
>
>
Re: Review Request 43704: SENTRY-1042: Create CommonPolicy for
external component
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
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 (updated)
-----
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