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 2015/03/09 03:50:21 UTC
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/
-----------------------------------------------------------
(Updated March 9, 2015, 2:50 a.m.)
Review request for sentry.
Repository: sentry
Description
-------
Update SentryService for import/export feature
Diffs (updated)
-----
sentry-provider/sentry-provider-db/pom.xml 6116cd5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 7a9f0df
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 44681ca
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b4c49da
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryTestImportExportUtil.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFileConstants.java b2bc531
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java f303294
Diff: https://reviews.apache.org/r/31071/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/
-----------------------------------------------------------
(Updated June 19, 2015, 1:32 a.m.)
Review request for sentry and Anne Yu.
Repository: sentry
Description
-------
Update SentryService for import/export feature
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 05cbfb6
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java c3c1907
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 30792f3
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
Diff: https://reviews.apache.org/r/31071/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Colin Ma <ju...@intel.com>.
> On June 17, 2015, 7:48 p.m., Anne Yu wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java, line 76
> > <https://reviews.apache.org/r/31071/diff/5/?file=976386#file976386line76>
> >
> > Instead of test1/2/3 could it be possible to give it a meaningful name?
For this problem, I'm sorry to use these confusing method name, but I use a lot of commets before each method to describe the purpose of the test case.
> On June 17, 2015, 7:48 p.m., Anne Yu wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java, line 382
> > <https://reviews.apache.org/r/31071/diff/5/?file=976386#file976386line382>
> >
> > One more test case: all, *, select, insert, however specify no-overwriting, so the permissions will be kept as before.
The overwriting for privilege and for import are the different things, for privilege, the all, * will always overwrite the select and insert for the same object.
For import, for example:
1st import:
role1=privilege1,privilege2
role2=privilege1
2nd import:
role1=privilege2,privilege3
role3=privilege1
if do the import with overwriting, the result will be:
role1=privilege2,privilege3
role2=privilege1
role3=privilege1
if without overwriting, the result will be:
role1=privilege1, privilege2,privilege3
role2=privilege1
role3=privilege1
I also add one test case about import select/insert first, and then import all, check the overwriting for the privilege.
- Colin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/#review88255
-----------------------------------------------------------
On June 19, 2015, 1:32 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31071/
> -----------------------------------------------------------
>
> (Updated June 19, 2015, 1:32 a.m.)
>
>
> Review request for sentry and Anne Yu.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Update SentryService for import/export feature
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 05cbfb6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java c3c1907
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 30792f3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/#review88255
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java (line 56)
<https://reviews.apache.org/r/31071/#comment140677>
White space
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java (line 72)
<https://reviews.apache.org/r/31071/#comment140678>
role2?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java (line 73)
<https://reviews.apache.org/r/31071/#comment140679>
role3?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java (line 76)
<https://reviews.apache.org/r/31071/#comment140680>
Instead of test1/2/3 could it be possible to give it a meaningful name?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java (line 142)
<https://reviews.apache.org/r/31071/#comment140686>
exceptedMappingData seems like policyFileMappingData1 + policyFileMappingData2. Maybe instead of redefining it, just combinte policyFileMappingData1 and 2.
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java (line 161)
<https://reviews.apache.org/r/31071/#comment140688>
Call import twice, and there has duplicate (or overlapping) groups;
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java (line 382)
<https://reviews.apache.org/r/31071/#comment140697>
One more test case: all, *, select, insert, however specify no-overwriting, so the permissions will be kept as before.
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java (line 383)
<https://reviews.apache.org/r/31071/#comment140698>
One more test case: with overlapping privileges, all,*, shadowing insert,select, and specify overwriting in importPolicy.
- Anne Yu
On June 2, 2015, 8:32 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31071/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 8:32 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Update SentryService for import/export feature
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 05cbfb6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java c3c1907
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 30792f3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/#review88253
-----------------------------------------------------------
Ship it!
Ship It!
- Sravya Tirukkovalur
On June 2, 2015, 8:32 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31071/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 8:32 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Update SentryService for import/export feature
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 05cbfb6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java c3c1907
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 30792f3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/
-----------------------------------------------------------
(Updated June 2, 2015, 8:32 a.m.)
Review request for sentry.
Repository: sentry
Description
-------
Update SentryService for import/export feature
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 05cbfb6
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java c3c1907
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 30792f3
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
Diff: https://reviews.apache.org/r/31071/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Colin Ma <ju...@intel.com>.
> On June 1, 2015, 8:24 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java, line 896
> > <https://reviews.apache.org/r/31071/diff/4/?file=964676#file964676line896>
> >
> > These utility functions should not be duplicated as much as possible and should stay in utility classes or SentryStore.
Thanks for the comments, create a new utility class SentryServiceUtil for these utility functions.
- Colin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/#review85683
-----------------------------------------------------------
On June 2, 2015, 8:32 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31071/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 8:32 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Update SentryService for import/export feature
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 05cbfb6
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java c3c1907
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 30792f3
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/#review85683
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
<https://reviews.apache.org/r/31071/#comment137387>
typo: poicy => policy?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
<https://reviews.apache.org/r/31071/#comment137389>
These utility functions should not be duplicated as much as possible and should stay in utility classes or SentryStore.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
<https://reviews.apache.org/r/31071/#comment137895>
Same comment about utility functions consolidation as above.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
<https://reviews.apache.org/r/31071/#comment137388>
same typo "Poicy"
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
<https://reviews.apache.org/r/31071/#comment137928>
Again, utility function to serialize TPrivilege, probably should remain in SentryStore or some other utility class?
- Sravya Tirukkovalur
On May 20, 2015, 1:34 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31071/
> -----------------------------------------------------------
>
> (Updated May 20, 2015, 1:34 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Update SentryService for import/export feature
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 7a9f0df
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 44681ca
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 30792f3
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/
-----------------------------------------------------------
(Updated May 20, 2015, 1:34 a.m.)
Review request for sentry.
Repository: sentry
Description
-------
Update SentryService for import/export feature
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 7a9f0df
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 44681ca
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 30792f3
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
Diff: https://reviews.apache.org/r/31071/diff/
Testing
-------
Thanks,
Colin Ma
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Colin Ma <ju...@intel.com>.
> On May 12, 2015, 4:49 a.m., Prasad Mujumdar wrote:
> > sentry-provider/sentry-provider-db/pom.xml, line 97
> > <https://reviews.apache.org/r/31071/diff/3/?file=888611#file888611line97>
> >
> > Can we avoid this. It's better to keep the providers isolated. If there's some common method, then please consider moving that to provider-common
Create new JIRA, sentry-740: Move the class PolicyFileConstants and KeyValue to provider-common.
> On May 12, 2015, 4:49 a.m., Prasad Mujumdar wrote:
> > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFileConstants.java, line 27
> > <https://reviews.apache.org/r/31071/diff/3/?file=888617#file888617line27>
> >
> > Perhaps this class could be moved to common package
Move the class PolicyFileConstants and KeyValue to provider-common
- Colin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/#review83288
-----------------------------------------------------------
On March 9, 2015, 2:50 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31071/
> -----------------------------------------------------------
>
> (Updated March 9, 2015, 2:50 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Update SentryService for import/export feature
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/pom.xml 6116cd5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 7a9f0df
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 44681ca
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b4c49da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryTestImportExportUtil.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFileConstants.java b2bc531
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java f303294
>
> Diff: https://reviews.apache.org/r/31071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/#review83288
-----------------------------------------------------------
As mentioned in the other subtask reviews, it would be very useful to provide an overwrite option for import (ie, replace the existing policies with the new ones).
Also we should add this for generalized privilege model as well, can be addressed via a separate task or followup ticket.
Are we audting the import and export actions on Sentry service side ?
A few additional comments/suggestions below
sentry-provider/sentry-provider-db/pom.xml
<https://reviews.apache.org/r/31071/#comment134195>
Junit should be test scope
sentry-provider/sentry-provider-db/pom.xml
<https://reviews.apache.org/r/31071/#comment134196>
Can we avoid this. It's better to keep the providers isolated. If there's some common method, then please consider moving that to provider-common
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
<https://reviews.apache.org/r/31071/#comment134321>
Shouldn't the Export be restricted to admin users ?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
<https://reviews.apache.org/r/31071/#comment134322>
Same as previous, import should be restricted to admin users
sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFileConstants.java
<https://reviews.apache.org/r/31071/#comment134323>
Perhaps this class could be moved to common package
- Prasad Mujumdar
On March 9, 2015, 2:50 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31071/
> -----------------------------------------------------------
>
> (Updated March 9, 2015, 2:50 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Update SentryService for import/export feature
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/pom.xml 6116cd5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 7a9f0df
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 44681ca
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b4c49da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryTestImportExportUtil.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFileConstants.java b2bc531
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java f303294
>
> Diff: https://reviews.apache.org/r/31071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/#review75653
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
<https://reviews.apache.org/r/31071/#comment122864>
why not convert Map<String, Set<String>> to Map<TSentryGroup, Set<String>? If we don't do this, we can reduce many efforts.
- Xiaomeng Huang
On March 9, 2015, 2:50 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31071/
> -----------------------------------------------------------
>
> (Updated March 9, 2015, 2:50 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Update SentryService for import/export feature
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/pom.xml 6116cd5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 7a9f0df
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 44681ca
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b4c49da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryTestImportExportUtil.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFileConstants.java b2bc531
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java f303294
>
> Diff: https://reviews.apache.org/r/31071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>
Re: Review Request 31071: SENTRY-622: Update SentryService for
import/export feature
Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31071/#review75668
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
<https://reviews.apache.org/r/31071/#comment122892>
Sorry for the typo, I means could we re-use Map<String, Set<String>> rather than convert to Map<TSentryGroup, Set<String>>?
- Xiaomeng Huang
On March 9, 2015, 2:50 a.m., Colin Ma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31071/
> -----------------------------------------------------------
>
> (Updated March 9, 2015, 2:50 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Update SentryService for import/export feature
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/pom.xml 6116cd5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java 7a9f0df
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java 44681ca
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b4c49da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/tools/SentryTestImportExportUtil.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java PRE-CREATION
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFileConstants.java b2bc531
> sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java f303294
>
> Diff: https://reviews.apache.org/r/31071/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Colin Ma
>
>