----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34756/#review102115 -----------------------------------------------------------
Ship it! Thanks for adding tests Anne! Overall looks good to me. Left minor comments. sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 68) <https://reviews.apache.org/r/34756/#comment159683> Nit: I see that we are using execBatch just for ensuring setup is good. But we should err on the side of throwing exceptions than logging them as errors when ever possible. sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 89) <https://reviews.apache.org/r/34756/#comment159670> Do we want to validate the rows in the resultset? sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 164) <https://reviews.apache.org/r/34756/#comment159642> Nit: I would be a little cautious when we want to skip tests, as we never investigate skipped tests in a green run :-) sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 183) <https://reviews.apache.org/r/34756/#comment159657> user? sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 185) <https://reviews.apache.org/r/34756/#comment159658> user? sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 241) <https://reviews.apache.org/r/34756/#comment159674> Why only on unmanagedcluster? - Sravya Tirukkovalur On Oct. 9, 2015, 4:06 a.m., Anne Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34756/ > ----------------------------------------------------------- > > (Updated Oct. 9, 2015, 4:06 a.m.) > > > Review request for sentry, Li Li, Lenni Kuff, and Sravya Tirukkovalur. > > > Bugs: SENTRY-748 > https://issues.apache.org/jira/browse/SENTRY-748 > > > Repository: sentry > > > Description > ------- > > Add new test case to validate complex view privilege: view by union tables, > view from another view, view with subquery and view by joining tables; > > Resent the cr. > > > Diffs > ----- > > sentry-tests/sentry-tests-hive/pom.xml > 7ee5378fdd2af383c4dc9b7f692d60b9c1173038 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/34756/diff/ > > > Testing > ------- > > https://builds.apache.org/job/PreCommit-SENTRY-Build/608/, seems like tests > all passed now. > > > Thanks, > > Anne Yu > >
