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




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
Lines 1386 (patched)
<https://reviews.apache.org/r/67649/#comment287913>

    This line has an unecessary extra ()



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
Lines 1402-1415 (patched)
<https://reviews.apache.org/r/67649/#comment287912>

    We need to catch the same exceptions that sentry_sync_notifications() 
throws.
    
    Btw, if we catch a sync exception, then are we going to ignore the 
ownership grant?



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
Lines 1453 (patched)
<https://reviews.apache.org/r/67649/#comment287914>

    'hive' user should not be hard-coded. Find a way to obtain the user that 
the Sentry server is using instead.



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/SentryPolicyStorePlugin.java
Lines 70 (patched)
<https://reviews.apache.org/r/67649/#comment287911>

    Is this a compatible change? I see that implementations of this interface 
can be configured in the sentry-site.xml. Should we keep the method 
compatibility?
    
    Same question for the next method.


- Sergio Pena


On June 19, 2018, 3:50 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67649/
> -----------------------------------------------------------
> 
> (Updated June 19, 2018, 3:50 p.m.)
> 
> 
> Review request for sentry, Na Li and Sergio Pena.
> 
> 
> Bugs: SENTRY-2241
>     https://issues.apache.org/jira/browse/SENTRY-2241
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Sentry has SentrySyncHMSNotificationsPostEventListener which is added a post 
> listener in HMS. This listener should be extended to get the owner 
> information of tables and databases. Based on these notifications owner 
> privileges are granted/revoked.
> 
> 
> Diffs
> -----
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryOwnerInfo.java
>  PRE-CREATION 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  b5e01e4c21473523b494cc624318b73ec5732408 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  5424bff67bc31d3f4ed2300531706e062f82b9ae 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  7f97ff7f054f797757aac94e243acdc5ee1127a2 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/SentryPolicyStorePlugin.java
>  52f25dc15d5b6d5e3ef1caa02baec8ed2923e290 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/CounterWait.java
>  d8c82970b56b1599a07f0e26edab8ed3d59b9948 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  f0e373a9aa5342d1b507e8b192cfdbc444242227 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java
>  6bfe872320d255acdb10f69e09c19623c04d8951 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  c056446262ddcf61db307eafbb785eac42973c80 
> 
> 
> Diff: https://reviews.apache.org/r/67649/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Reply via email to