-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68444/#review207683
-----------------------------------------------------------




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
Line 268 (original), 268 (patched)
<https://reviews.apache.org/r/68444/#comment291166>

    What is impact to throw this exception to HMS? 
    
    In 
https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java#L1068,
    
    There is no catch exception for each listener call. Throwing exception in 
sentry post event listener will cause following listener not called, and any 
other code after in finally not called. 
    
    Can we accept this behavior?


- Na Li


On Aug. 21, 2018, 5:45 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68444/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2018, 5:45 p.m.)
> 
> 
> Review request for sentry, Na Li and Sergio Pena.
> 
> 
> Bugs: SENTRY-2318
>     https://issues.apache.org/jira/browse/SENTRY-2318
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> If the grant/revoke of owner privilege fails for any reason it should logged 
> be as an error in the hive logs otherwise this will go un-noticed.
> 
> 
> Diffs
> -----
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
>  c37f3706092f2239e45e1cef2f31cb8ab1c886f5 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/metastore/TestSentrySyncHMSNotificationsPostEventListener.java
>  98de8e36adfd790996aba8e61a164d85e80bc006 
> 
> 
> Diff: https://reviews.apache.org/r/68444/diff/1/
> 
> 
> Testing
> -------
> 
> Updated the tests to cover the change.
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Reply via email to