> On May 16, 2017, 1:34 a.m., Alexander Kolbasov wrote: > > sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java > > Lines 154 (patched) > > <https://reviews.apache.org/r/59120/diff/5/?file=1716364#file1716364line154> > > > > Can you add a comment explaining what cache should be updated and why > > this is the value used? Also, please specify units for this delay.
will do > On May 16, 2017, 1:34 a.m., Alexander Kolbasov wrote: > > sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java > > Line 1216 (original), 1220 (patched) > > <https://reviews.apache.org/r/59120/diff/5/?file=1716364#file1716364line1221> > > > > This is rather unreliable - could there be some other way to wait for > > the condition which isn't time based? The time is long enough for the info to reach HDFS (I will add comment on the value of WAIT_BEFORE_TESTVERIFY) if it works. So it is reliable. It is hard to guarantee that in other ways. > On May 16, 2017, 1:34 a.m., Alexander Kolbasov wrote: > > sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java > > Line 1342 (original), 1346 (patched) > > <https://reviews.apache.org/r/59120/diff/5/?file=1716364#file1716364line1347> > > > > Will it cause all HDFS tests to run longer? Yes for all tests using "Thread.sleep(WAIT_BEFORE_TESTVERIFY)". But what is the alternative? > On May 16, 2017, 1:34 a.m., Alexander Kolbasov wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java > > Line 152 (original), 153 (patched) > > <https://reviews.apache.org/r/59120/diff/5/?file=1716366#file1716366line153> > > > > Can you put your comment in the code? What are units for this wait? How > > did you come up with this formula? the unit is ms. I will add comments. > On May 16, 2017, 1:34 a.m., Alexander Kolbasov wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java > > Lines 156 (patched) > > <https://reviews.apache.org/r/59120/diff/5/?file=1716366#file1716366line156> > > > > What is this one and why it is needed? What are units? Waiting at the end of the test cleanup fixed the test failure. Without this wait, test fails from time to time. I will add more detailed comment - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59120/#review175051 ----------------------------------------------------------- On May 12, 2017, 3:40 a.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59120/ > ----------------------------------------------------------- > > (Updated May 12, 2017, 3:40 a.m.) > > > Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and > Sergio Pena. > > > Bugs: sentry-1757 > https://issues.apache.org/jira/browse/sentry-1757 > > > Repository: sentry > > > Description > ------- > > use hive configuration to check. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java > 431c7fe > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 9880c40 > > sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java > 1530eb2 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java > 7d128b7 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java > 32acc65 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java > 548dcf8 > > > Diff: https://reviews.apache.org/r/59120/diff/5/ > > > Testing > ------- > > integration test TestHDFSIntegrationEnd2End > > > Thanks, > > Na Li > >
