----------------------------------------------------------- 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 > >
