----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42385/#review115569 -----------------------------------------------------------
Ship it! nice, thanks for the extra test coverage. sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java (line 255) <https://reviews.apache.org/r/42385/#comment176559> Add comment on why you set this to be true sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java (line 991) <https://reviews.apache.org/r/42385/#comment176557> Should this be Assert.assertTrue (so that it triggers a failure)? Alternatively, it might be more clear to just do Assert.fail() sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java (line 1005) <https://reviews.apache.org/r/42385/#comment176560> nit: either change "event" -> "events" or change "fail," -> "fails", sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java (line 1044) <https://reviews.apache.org/r/42385/#comment176558> Move the create table statement out of the try block (since we expect the table creation to succeed) - Lenni Kuff On Jan. 20, 2016, 9:18 p.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42385/ > ----------------------------------------------------------- > > (Updated Jan. 20, 2016, 9:18 p.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Change-Id: Ia054eab294d712641ea9e6dc58521d41c2e65ef9 > > Sentry plugin for hdfs path sync is not handling the error case properly. > When even such as createTable is not successful, the path should not be > updated to Sentry server. > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java > 3c8ad1f6787623bf95d0ba53de9eb09223f9999b > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java > 5a93ba0108bbe542e8a294ef43b96fa0c718eda5 > > Diff: https://reviews.apache.org/r/42385/diff/ > > > Testing > ------- > > Added e2e test TestHDFSIntegration.testCreationTableFailure, > testAddPartitionFailure, testDropTableFailure and testDropPartitionFailure. > > > Thanks, > > Hao Hao > >
