You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org> on 2017/12/05 21:09:38 UTC
Re: Review Request 63958: SENTRY-2034: Add e2e tests for testing HMS
notification processing.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63958/
-----------------------------------------------------------
(Updated Dec. 5, 2017, 9:09 p.m.)
Review request for sentry, Na Li and Sergio Pena.
Changes
-------
adddresed review comments.
Bugs: SENTRY-2034
https://issues.apache.org/jira/browse/SENTRY-2034
Repository: sentry
Description
-------
Currently, there are no e2e tests that test the functionality of pulling the notifications from HMS and processing them. Which include updating the update the permissions based on the HMS updates.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java 21383046f8a5b7386cfe9f6f62c0119542a73382
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessing.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnCreate.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnDrop.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java 33ace57134b20189aa10bee48f6aae765a1a1830
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java c8eef09ed61e11583838a3dfd3117a9478df23ba
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 645fc354c3b4e5b21456ee9a471253ffa4624f17
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java a911e2f50f63db6a99996ce44a74ad6bcc2a82b0
Diff: https://reviews.apache.org/r/63958/diff/3/
Changes: https://reviews.apache.org/r/63958/diff/2-3/
Testing
-------
Made sure that new tests added and all the older tests pass.
Thanks,
kalyan kumar kalvagadda
Re: Review Request 63958: SENTRY-2034: Add e2e tests for testing HMS
notification processing.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63958/#review193972
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessing.java
Lines 21-22 (patched)
<https://reviews.apache.org/r/63958/#comment272665>
There are a few commented lines here we should remove.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessing.java
Lines 73 (patched)
<https://reviews.apache.org/r/63958/#comment272667>
Is there a better way to wait for HMS processing instead of waiting 5s? These 5s will increment testing time and believe hte processing is faster than than.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java
Lines 20-33 (patched)
<https://reviews.apache.org/r/63958/#comment272664>
There are a few commented lines here we should remove.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java
Lines 45 (patched)
<https://reviews.apache.org/r/63958/#comment272666>
This method name confuses a little the verifyPrivilegeDropped(). Can we rename it to verifyAllPrivilegesDropped()?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
Lines 52-54 (patched)
<https://reviews.apache.org/r/63958/#comment272663>
I see it exists a hdfsSyncEnabled variable on TestHDFSIntegrationBase which sets these properties. Can we use that variable instead?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
Lines 52-54 (patched)
<https://reviews.apache.org/r/63958/#comment272661>
I see it exists a hdfsSyncEnabled variable on TestHDFSIntegrationBase which sets these properties. Can we use that variable instead?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java
Lines 27-29 (patched)
<https://reviews.apache.org/r/63958/#comment272662>
I see it exists a hdfsSyncEnabled variable on TestHDFSIntegrationBase which sets these properties. Can we use that variable instead?
- Sergio Pena
On Dec. 5, 2017, 9:09 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63958/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 9:09 p.m.)
>
>
> Review request for sentry, Na Li and Sergio Pena.
>
>
> Bugs: SENTRY-2034
> https://issues.apache.org/jira/browse/SENTRY-2034
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently, there are no e2e tests that test the functionality of pulling the notifications from HMS and processing them. Which include updating the update the permissions based on the HMS updates.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java 21383046f8a5b7386cfe9f6f62c0119542a73382
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessing.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnCreate.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java 33ace57134b20189aa10bee48f6aae765a1a1830
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java c8eef09ed61e11583838a3dfd3117a9478df23ba
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 645fc354c3b4e5b21456ee9a471253ffa4624f17
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java a911e2f50f63db6a99996ce44a74ad6bcc2a82b0
>
>
> Diff: https://reviews.apache.org/r/63958/diff/3/
>
>
> Testing
> -------
>
> Made sure that new tests added and all the older tests pass.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 63958: SENTRY-2034: Add e2e tests for testing HMS
notification processing.
Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63958/#review193939
-----------------------------------------------------------
Ship it!
Ship It!
- Na Li
On Dec. 5, 2017, 9:09 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63958/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 9:09 p.m.)
>
>
> Review request for sentry, Na Li and Sergio Pena.
>
>
> Bugs: SENTRY-2034
> https://issues.apache.org/jira/browse/SENTRY-2034
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently, there are no e2e tests that test the functionality of pulling the notifications from HMS and processing them. Which include updating the update the permissions based on the HMS updates.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java 21383046f8a5b7386cfe9f6f62c0119542a73382
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessing.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnCreate.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java 33ace57134b20189aa10bee48f6aae765a1a1830
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java c8eef09ed61e11583838a3dfd3117a9478df23ba
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 645fc354c3b4e5b21456ee9a471253ffa4624f17
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java a911e2f50f63db6a99996ce44a74ad6bcc2a82b0
>
>
> Diff: https://reviews.apache.org/r/63958/diff/3/
>
>
> Testing
> -------
>
> Made sure that new tests added and all the older tests pass.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 63958: SENTRY-2034: Add e2e tests for testing HMS
notification processing.
Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63958/#review195470
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On Jan. 11, 2018, 7:44 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63958/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2018, 7:44 p.m.)
>
>
> Review request for sentry, Na Li and Sergio Pena.
>
>
> Bugs: SENTRY-2034
> https://issues.apache.org/jira/browse/SENTRY-2034
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently, there are no e2e tests that test the functionality of pulling the notifications from HMS and processing them. Which include updating the update the permissions based on the HMS updates.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java 21383046f8a5b7386cfe9f6f62c0119542a73382
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessing.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnCreate.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java 33ace57134b20189aa10bee48f6aae765a1a1830
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java c8eef09ed61e11583838a3dfd3117a9478df23ba
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java a909b913e9d2342a7f76a886d3fdabdb1e309b9f
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java cc0449b1d1d0291049cb1fafebb6d752dd426fbc
>
>
> Diff: https://reviews.apache.org/r/63958/diff/4/
>
>
> Testing
> -------
>
> Made sure that new tests added and all the older tests pass.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 63958: SENTRY-2034: Add e2e tests for testing HMS
notification processing.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63958/
-----------------------------------------------------------
(Updated Jan. 11, 2018, 7:44 p.m.)
Review request for sentry, Na Li and Sergio Pena.
Changes
-------
Addressed review comments from sergio.
Bugs: SENTRY-2034
https://issues.apache.org/jira/browse/SENTRY-2034
Repository: sentry
Description
-------
Currently, there are no e2e tests that test the functionality of pulling the notifications from HMS and processing them. Which include updating the update the permissions based on the HMS updates.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java 21383046f8a5b7386cfe9f6f62c0119542a73382
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessing.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnCreate.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnDrop.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java 33ace57134b20189aa10bee48f6aae765a1a1830
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java c8eef09ed61e11583838a3dfd3117a9478df23ba
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java a909b913e9d2342a7f76a886d3fdabdb1e309b9f
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java cc0449b1d1d0291049cb1fafebb6d752dd426fbc
Diff: https://reviews.apache.org/r/63958/diff/4/
Changes: https://reviews.apache.org/r/63958/diff/3-4/
Testing
-------
Made sure that new tests added and all the older tests pass.
Thanks,
kalyan kumar kalvagadda
Re: Review Request 63958: SENTRY-2034: Add e2e tests for testing HMS
notification processing.
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63958/#review191548
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
Lines 52-54 (patched)
<https://reviews.apache.org/r/63958/#comment269394>
Values for these properties will vary based on the configuration that is why I moved them out of TestHDFIntegrationBase.
I did not want to refoctor the class. I've limited the changes that I actually need.
- kalyan kumar kalvagadda
On Dec. 5, 2017, 9:09 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63958/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 9:09 p.m.)
>
>
> Review request for sentry, Na Li and Sergio Pena.
>
>
> Bugs: SENTRY-2034
> https://issues.apache.org/jira/browse/SENTRY-2034
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Currently, there are no e2e tests that test the functionality of pulling the notifications from HMS and processing them. Which include updating the update the permissions based on the HMS updates.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java 21383046f8a5b7386cfe9f6f62c0119542a73382
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessing.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnCreate.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutSyncOnDrop.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java 33ace57134b20189aa10bee48f6aae765a1a1830
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java c8eef09ed61e11583838a3dfd3117a9478df23ba
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 645fc354c3b4e5b21456ee9a471253ffa4624f17
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationWithHA.java a911e2f50f63db6a99996ce44a74ad6bcc2a82b0
>
>
> Diff: https://reviews.apache.org/r/63958/diff/3/
>
>
> Testing
> -------
>
> Made sure that new tests added and all the older tests pass.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>