> On April 25, 2016, 12:40 a.m., Hao Hao wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java, > > line 124 > > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line124> > > > > What are the HDFS specif tearDown here?Will the tearDown be the same > > when enableHDFSAcls is true?
Previously thought about cleaning up all files/dirs under prefix path but forgot to add. Will add them in the next patch. Thanks for pointing it out. > On April 25, 2016, 12:40 a.m., Hao Hao wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java, > > line 173 > > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line173> > > > > Could you add the comments here to explain the retry logic for getting > > the acls? Also for the recursive verify logic? It takes some time for acls to be synced up on hdfs files/dirs; Especially for these on external prefix path. On my testing cluster (2x large), takes >= 10s. > On April 25, 2016, 12:40 a.m., Hao Hao wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java, > > line 204 > > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line204> > > > > Commnet to explain why we need this? In case the roles haven't be cleaned up correctly from previous runs. Don't want tests to fail because of it. Since most of the time, roles don't exit, might throw exception, so just put all these logic in a convenience method. > On April 25, 2016, 12:40 a.m., Hao Hao wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java, > > line 208 > > <https://reviews.apache.org/r/46542/diff/2/?file=1356092#file1356092line208> > > > > Log the exception here? OK. > On April 25, 2016, 12:40 a.m., Hao Hao wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java, > > line 47 > > <https://reviews.apache.org/r/46542/diff/2/?file=1356093#file1356093line47> > > > > Do we need to make the max_num_of_groups configurable for different > > value testing? e.g make it > 32? Yeah, for table dir, it will have max_num_of_groups*2 acl entries; for partition dir, it will have max_num_of_groups*3 acl entries. More acl entries put into the dir, longer time to sync up acls across components. So I chose 20. > On April 25, 2016, 12:40 a.m., Hao Hao wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java, > > line 68 > > <https://reviews.apache.org/r/46542/diff/2/?file=1356093#file1356093line68> > > > > How about test for external table outside of prefix? To check the ACLs > > are not changed by Sentry? You meant negative test? Could add one for sure. Will do it in the next patch. - Anne ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46542/#review130332 ----------------------------------------------------------- On April 21, 2016, 11:47 p.m., Anne Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46542/ > ----------------------------------------------------------- > > (Updated April 21, 2016, 11:47 p.m.) > > > Review request for sentry, Hao Hao and Lenni Kuff. > > > Bugs: SENTRY-583 > https://issues.apache.org/jira/browse/SENTRY-583 > > > Repository: sentry > > > Description > ------- > > Add one boundary HDFS SyncUp test to run on a real cluster; > Also add some basic APIs for createing more HDFS SyncUp tests. > > > Diffs > ----- > > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsBase.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestDbHdfsMaxGroups.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > 8515a2ba1fc08104a788ea054f2b871905359273 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/DFSFactory.java > e1881b4bfaa5923fd0dab2096f670b22b944f116 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java > 77af4329642807283e6cd3d2e3be837c5d3e0dfb > > Diff: https://reviews.apache.org/r/46542/diff/ > > > Testing > ------- > > Tested on a unmanaged real cluster. > > -------------------------------------------------------- > T E S T S > ------------------------------------------------------- > Running org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups > Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 50.615 sec - > in org.apache.sentry.tests.e2e.hdfs.TestDbHdfsMaxGroups > > Results : > > Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 > > > Thanks, > > Anne Yu > >
