> On July 11, 2016, 6:36 p.m., Hao Hao wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java, > > line 218 > > <https://reviews.apache.org/r/49397/diff/8/?file=1440295#file1440295line218> > > > > Other than create view, will it be possible for some other DDL trigger > > createTable but without location? If so, we may want not to throw exception > > everytime if location is null. And can log which event does not have > > location.
That is aready being handled. if(!tableEvent.getTable().getTableType().equals(TableType.VIRTUAL_VIEW.name())) { String location = tableEvent.getTable().getSd().getLocation(); if (location == null || location.isEmpty()) { throw new MetaException("CreateTableEvent has invalid location: tableEvent " + tableEvent); } } > On July 11, 2016, 6:36 p.m., Hao Hao wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java, > > line 281 > > <https://reviews.apache.org/r/49397/diff/8/?file=1440295#file1440295line281> > > > > Could you comment on why do we need database information? If would be > > good to comment on what we need for each event. DbName is part of table identity, as we do not have a table id. So we need it for all table operations. - Sravya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49397/#review141740 ----------------------------------------------------------- On July 9, 2016, 8:38 a.m., Sravya Tirukkovalur wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49397/ > ----------------------------------------------------------- > > (Updated July 9, 2016, 8:38 a.m.) > > > Review request for sentry, Anne Yu, Colin McCabe, Hao Hao, and Nachiket > Vaidya. > > > Bugs: Sentry-1329 > https://issues.apache.org/jira/browse/Sentry-1329 > > > Repository: sentry > > > Description > ------- > > This patch adapts SentryMetaStorePostEventListener to write HMS notification > logs. Also, > 1. Implements the SentryJSONMessageFactory to add custom information in the > notification log entry message, which includes > 1.1. Implementing Message class for each message type > 1.2. Implementing a deserializer > 2. Implements JSONAlterPartitionMessage and JSONAlterTableMessage to work > around the issue in Hive 1.1.0. > 3. Testing: > 3.1. Sentry functionality: TestSentryListenerSentryDeserializer to verify > functionality using Sentry's SentryMetastorePostEventListener and Sentry > Notification log deserializer. > 3.2. TestDbNotificationListenerSentryDeserializer uses Hive's > DbNotificationListener and Sentry's JSON deserializeri. This would make sure > Sentry is able to read the Notification logs written by Hive's > DBNotificationListener > 3.3. TestSentryListenerInBuiltDeserializer uses Sentry's > SentryMetastorePostEventListener and Hive's inbuilt Notification log > deserializer: This would make sure Sentry is not breaking other users of > NotificationLog who might be using Hive's in built serializer > > > > Change-Id: I680beb6db4e534bb0a9e6ee042ea0d4f33f0943f > > update > > Change-Id: I6ce8846a801b3effc6a851ab3ce6f866b4a0c23e > > > Diffs > ----- > > sentry-binding/sentry-binding-hive/pom.xml > 07aaae3bac34582fe8222ff166a52c1e208b223a > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java > d12ac151ddbf35d612fee3f869d602d6cdf54aa8 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAddPartitionMessage.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateDatabaseMessage.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateTableMessage.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropDatabaseMessage.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropTableMessage.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java > PRE-CREATION > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java > 767bcbe02e0d511f52bc869e7b6a1ee1e6584a5a > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > 2c4948e9d100f2cf0cb5b7772489194c62b8a857 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java > b72e317621610eababf5d01886bc00dca9f5e7ec > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestDbNotificationListenerSentryDeserializer.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestHMSNotificationLogUsingDBNotificationListener.java > 0b328d402d005d6f858e87fbe64eedd5f9f1a092 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerInBuiltDeserializer.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/49397/diff/ > > > Testing > ------- > > Added new tests > > > Thanks, > > Sravya Tirukkovalur > >