> 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
> 
>

Reply via email to