You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sravya Tirukkovalur <sr...@cloudera.com> on 2013/10/04 01:50:43 UTC
Review Request 14474: SENTRY-10
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14474/
-----------------------------------------------------------
Review request for sentry and Shreepadma Venugopalan.
Repository: sentry
Description
-------
Tests were very tightly coupled with which file system it uses. It is nice to have all the tests utilizing miniDFS instead of localFS for hive warehouse. It would also make it possible to use any other FS implementations like Cluster HDFS, so that we can run the end to end tests with little changes on real clusters. MiniDFS and ClusterDFS implementations are added as part of this patch. LocalFS implementation will be added with a follow on JIRA.
Diffs
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java cae15ae
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java ba05044
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticDFS.java f670f89
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticLocalFS.java 3954b9a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 85ddc67
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestEndToEnd.java ff8fd9c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java 304b2af
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataObjectRetrieval.java cd8daf2
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataPermissions.java 57b9532
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java 5d53154
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDBConfiguration.java dbe1928
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDatabasePolicyFile.java 9a03728
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 2b309d8
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ae0688a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java 3194790
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java a5c36ad
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java cd398da
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java d830c21
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java 7d1ce6f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 4dc9b88
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 102c6cf
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 653d2ec
Diff: https://reviews.apache.org/r/14474/diff/
Testing
-------
All tests pass
Thanks,
Sravya Tirukkovalur
Re: Review Request 14474: SENTRY-10
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14474/
-----------------------------------------------------------
(Updated Oct. 7, 2013, 11:33 p.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Updated the patch with suggestions.
- Removed ClusterDFS
- DFSFactory.create accepts a type
- Other minor nits
Repository: sentry
Description
-------
Tests were very tightly coupled with which file system it uses. It is nice to have all the tests utilizing miniDFS instead of localFS for hive warehouse. It would also make it possible to use any other FS implementations like Cluster HDFS, so that we can run the end to end tests with little changes on real clusters. MiniDFS implementation is added as part of this patch. ClusterDFS and LocalFS implementation will be added with follow on JIRAs.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java cae15ae
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java ba05044
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticDFS.java f670f89
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticLocalFS.java 3954b9a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 85ddc67
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestEndToEnd.java ff8fd9c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java 304b2af
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataObjectRetrieval.java cd8daf2
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataPermissions.java 57b9532
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java 5d53154
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDBConfiguration.java dbe1928
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDatabasePolicyFile.java 9a03728
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 2b309d8
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ae0688a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java 3194790
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java a5c36ad
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java cd398da
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java d830c21
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java 7d1ce6f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 4dc9b88
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 102c6cf
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 653d2ec
Diff: https://reviews.apache.org/r/14474/diff/
Testing
-------
All tests pass
Thanks,
Sravya Tirukkovalur
Re: Review Request 14474: SENTRY-10
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14474/
-----------------------------------------------------------
(Updated Oct. 7, 2013, 10:13 p.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Moving ClusterDFS to another follow on JIRA, as it is a new feature, and retaining this jira to be a refactoring JIRA.
Repository: sentry
Description (updated)
-------
Tests were very tightly coupled with which file system it uses. It is nice to have all the tests utilizing miniDFS instead of localFS for hive warehouse. It would also make it possible to use any other FS implementations like Cluster HDFS, so that we can run the end to end tests with little changes on real clusters. MiniDFS implementation is added as part of this patch. ClusterDFS and LocalFS implementation will be added with follow on JIRAs.
Diffs
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java cae15ae
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java ba05044
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticDFS.java f670f89
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticLocalFS.java 3954b9a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 85ddc67
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestEndToEnd.java ff8fd9c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java 304b2af
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataObjectRetrieval.java cd8daf2
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataPermissions.java 57b9532
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java 5d53154
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDBConfiguration.java dbe1928
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDatabasePolicyFile.java 9a03728
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 2b309d8
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ae0688a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java 3194790
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java a5c36ad
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java cd398da
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java d830c21
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java 7d1ce6f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 4dc9b88
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 102c6cf
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 653d2ec
Diff: https://reviews.apache.org/r/14474/diff/
Testing
-------
All tests pass
Thanks,
Sravya Tirukkovalur
Re: Review Request 14474: SENTRY-10
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
> On Oct. 7, 2013, 5:37 p.m., Shreepadma Venugopalan wrote:
> > Overall comment - Wouldn't it be better to explicitly create a clusterDFS object instead of looking up a system property in the DFSFactory class? If you want to do this as a follow on, that's fine.
Can you please elaborate on "explicitly create a clusterDFS object"? I am not sure I understand that, so the idea here is to be able to run the tests against miniDFS or ClusterDFS depending on the system property.
Thanks for reviewing Shreepadma. I have an overall question, I just realized that we need to make some more changes relating to the clusterDFS, do you want me to make those changes and update a new patch, or do you think its better to create a separate jira for ClusterDFS?
Thanks!
- Sravya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14474/#review26720
-----------------------------------------------------------
On Oct. 3, 2013, 11:50 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14474/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2013, 11:50 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Tests were very tightly coupled with which file system it uses. It is nice to have all the tests utilizing miniDFS instead of localFS for hive warehouse. It would also make it possible to use any other FS implementations like Cluster HDFS, so that we can run the end to end tests with little changes on real clusters. MiniDFS and ClusterDFS implementations are added as part of this patch. LocalFS implementation will be added with a follow on JIRA.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java cae15ae
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java ba05044
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticDFS.java f670f89
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticLocalFS.java 3954b9a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 85ddc67
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestEndToEnd.java ff8fd9c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java 304b2af
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataObjectRetrieval.java cd8daf2
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataPermissions.java 57b9532
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java 5d53154
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDBConfiguration.java dbe1928
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDatabasePolicyFile.java 9a03728
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 2b309d8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ae0688a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java 3194790
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java a5c36ad
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java cd398da
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java d830c21
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java 7d1ce6f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 4dc9b88
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 102c6cf
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 653d2ec
>
> Diff: https://reviews.apache.org/r/14474/diff/
>
>
> Testing
> -------
>
> All tests pass
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 14474: SENTRY-10
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
> On Oct. 7, 2013, 5:37 p.m., Shreepadma Venugopalan wrote:
> > Overall comment - Wouldn't it be better to explicitly create a clusterDFS object instead of looking up a system property in the DFSFactory class? If you want to do this as a follow on, that's fine.
>
> Sravya Tirukkovalur wrote:
> Can you please elaborate on "explicitly create a clusterDFS object"? I am not sure I understand that, so the idea here is to be able to run the tests against miniDFS or ClusterDFS depending on the system property.
>
> Thanks for reviewing Shreepadma. I have an overall question, I just realized that we need to make some more changes relating to the clusterDFS, do you want me to make those changes and update a new patch, or do you think its better to create a separate jira for ClusterDFS?
>
> Thanks!
Instead of reading the system property in DFSFactory, create a new class called say TestInClusterMode or something that reads the system property and explicitly requests a cluserDFS object from the DFSFactory class.
"I have an overall question, I just realized that we need to make some more changes relating to the clusterDFS, do you want me to make those changes and update a new patch, or do you think its better to create a separate jira for ClusterDFS"
It depends on what those changes. The main purpose of this JIRA seems to be refactoring. If it falls under the same category, it probably belongs here.
- Shreepadma
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14474/#review26720
-----------------------------------------------------------
On Oct. 3, 2013, 11:50 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14474/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2013, 11:50 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Tests were very tightly coupled with which file system it uses. It is nice to have all the tests utilizing miniDFS instead of localFS for hive warehouse. It would also make it possible to use any other FS implementations like Cluster HDFS, so that we can run the end to end tests with little changes on real clusters. MiniDFS and ClusterDFS implementations are added as part of this patch. LocalFS implementation will be added with a follow on JIRA.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java cae15ae
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java ba05044
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticDFS.java f670f89
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticLocalFS.java 3954b9a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 85ddc67
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestEndToEnd.java ff8fd9c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java 304b2af
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataObjectRetrieval.java cd8daf2
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataPermissions.java 57b9532
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java 5d53154
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDBConfiguration.java dbe1928
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDatabasePolicyFile.java 9a03728
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 2b309d8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ae0688a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java 3194790
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java a5c36ad
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java cd398da
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java d830c21
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java 7d1ce6f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 4dc9b88
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 102c6cf
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 653d2ec
>
> Diff: https://reviews.apache.org/r/14474/diff/
>
>
> Testing
> -------
>
> All tests pass
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 14474: SENTRY-10
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
> On Oct. 7, 2013, 5:37 p.m., Shreepadma Venugopalan wrote:
> > Overall comment - Wouldn't it be better to explicitly create a clusterDFS object instead of looking up a system property in the DFSFactory class? If you want to do this as a follow on, that's fine.
>
> Sravya Tirukkovalur wrote:
> Can you please elaborate on "explicitly create a clusterDFS object"? I am not sure I understand that, so the idea here is to be able to run the tests against miniDFS or ClusterDFS depending on the system property.
>
> Thanks for reviewing Shreepadma. I have an overall question, I just realized that we need to make some more changes relating to the clusterDFS, do you want me to make those changes and update a new patch, or do you think its better to create a separate jira for ClusterDFS?
>
> Thanks!
>
> Shreepadma Venugopalan wrote:
> Instead of reading the system property in DFSFactory, create a new class called say TestInClusterMode or something that reads the system property and explicitly requests a cluserDFS object from the DFSFactory class.
>
> "I have an overall question, I just realized that we need to make some more changes relating to the clusterDFS, do you want me to make those changes and update a new patch, or do you think its better to create a separate jira for ClusterDFS"
>
> It depends on what those changes. The main purpose of this JIRA seems to be refactoring. If it falls under the same category, it probably belongs here.
The changes are related to ClusterDFS. As the intent of this JIRA is refactoring, I will move the ClusterDFS to another JIRA.
Thanks!
- Sravya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14474/#review26720
-----------------------------------------------------------
On Oct. 3, 2013, 11:50 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14474/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2013, 11:50 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Tests were very tightly coupled with which file system it uses. It is nice to have all the tests utilizing miniDFS instead of localFS for hive warehouse. It would also make it possible to use any other FS implementations like Cluster HDFS, so that we can run the end to end tests with little changes on real clusters. MiniDFS and ClusterDFS implementations are added as part of this patch. LocalFS implementation will be added with a follow on JIRA.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java cae15ae
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java ba05044
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticDFS.java f670f89
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticLocalFS.java 3954b9a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 85ddc67
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestEndToEnd.java ff8fd9c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java 304b2af
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataObjectRetrieval.java cd8daf2
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataPermissions.java 57b9532
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java 5d53154
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDBConfiguration.java dbe1928
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDatabasePolicyFile.java 9a03728
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 2b309d8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ae0688a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java 3194790
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java a5c36ad
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java cd398da
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java d830c21
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java 7d1ce6f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 4dc9b88
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 102c6cf
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 653d2ec
>
> Diff: https://reviews.apache.org/r/14474/diff/
>
>
> Testing
> -------
>
> All tests pass
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>
Re: Review Request 14474: SENTRY-10
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14474/#review26720
-----------------------------------------------------------
Overall comment - Wouldn't it be better to explicitly create a clusterDFS object instead of looking up a system property in the DFSFactory class? If you want to do this as a follow on, that's fine.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
<https://reviews.apache.org/r/14474/#comment52043>
nit: policyFileLocation
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java
<https://reviews.apache.org/r/14474/#comment52048>
nit: Is this comment still valid?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
<https://reviews.apache.org/r/14474/#comment52049>
nit: braces
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java
<https://reviews.apache.org/r/14474/#comment52050>
Shouldn't this be MiniDFS since the default seems to be miniDFS? default switch case should never be hit.
- Shreepadma Venugopalan
On Oct. 3, 2013, 11:50 p.m., Sravya Tirukkovalur wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14474/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2013, 11:50 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Tests were very tightly coupled with which file system it uses. It is nice to have all the tests utilizing miniDFS instead of localFS for hive warehouse. It would also make it possible to use any other FS implementations like Cluster HDFS, so that we can run the end to end tests with little changes on real clusters. MiniDFS and ClusterDFS implementations are added as part of this patch. LocalFS implementation will be added with a follow on JIRA.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithHiveServer.java cae15ae
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java ba05044
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticDFS.java f670f89
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticLocalFS.java 3954b9a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestCrossDbOps.java 85ddc67
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestEndToEnd.java ff8fd9c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java 304b2af
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataObjectRetrieval.java cd8daf2
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMetadataPermissions.java 57b9532
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestMovingToProduction.java 5d53154
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDBConfiguration.java dbe1928
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPerDatabasePolicyFile.java 9a03728
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegeAtTransform.java 2b309d8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtDatabaseScope.java ae0688a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtFunctionScope.java 3194790
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java a5c36ad
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java cd398da
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSandboxOps.java d830c21
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java 7d1ce6f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUriPermissions.java 4dc9b88
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestUserManagement.java 102c6cf
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/AbstractDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/ClusterDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 653d2ec
>
> Diff: https://reviews.apache.org/r/14474/diff/
>
>
> Testing
> -------
>
> All tests pass
>
>
> Thanks,
>
> Sravya Tirukkovalur
>
>