> On June 15, 2015, 11:12 p.m., Anne Yu wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPolicyImportExport.java,
> >  line 44
> > <https://reviews.apache.org/r/31074/diff/4/?file=976407#file976407line44>
> >
> >     Seems like PRIVILIEGE1...8 is derrived from testPolicyImport.ini. Could 
> > it be possible to combine these two definitions together? In this case, 
> > when make change to the first place will automatically reflect to the file. 
> > It will be easier to maintain the code.

Thanks for the comments, currently, all test data for TestPolicyImportExport 
are read from the .ini file, and the PRIVILIEGE1...8 are used for verification. 
The PRIVILIEGE1...8 are duplicated with the testPolicyImport.ini, to avoid 
this, maybe we can generate the testPolicyImport.ini by PolicyFile, but to test 
the import/export with file, it's better to use the testPolicyImport.ini as 
currently. For the code maintain, we have to update the both place if anything 
changed, but I think it's not a big problem for the test class.


- Colin


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


On June 2, 2015, 8:38 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31074/
> -----------------------------------------------------------
> 
> (Updated June 2, 2015, 8:38 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Update SentryConfigTool for import/export feature
> 
> 
> Diffs
> -----
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java
>  4388ca0 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPolicyImport.java
>  7ebc0e4 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPolicyImportExport.java
>  PRE-CREATION 
>   sentry-tests/sentry-tests-hive/src/test/resources/testPolicyImport.ini 
> PRE-CREATION 
>   sentry-tests/sentry-tests-hive/src/test/resources/testPolicyImportAdmin.ini 
> PRE-CREATION 
>   sentry-tests/sentry-tests-hive/src/test/resources/testPolicyImportError.ini 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31074/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Reply via email to