> On June 27, 2018, 12:50 p.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHmsEvent.java
> > Lines 224-233 (patched)
> > <https://reviews.apache.org/r/67749/diff/2/?file=2046227#file2046227line224>
> >
> >     As sentry doesn't changes to configurations at runtime, we doesn't have 
> > to do this every time.
> >     We can use constant that is initialized once and we can just use in 
> > setAuthorizable method.
> >     
> >     we need not pass the HiveAuthzConf while construncting SentryHmsEvent.

I have changed to code to get server name at once and use it in following 
calls. 

For class encapsulation and future extention, it is still better to provide 
HiveAuthzConf to SentryHmsEvent than for caller to get the configuration and 
provide to it as input.


- Na


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


On June 27, 2018, 3:57 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67749/
> -----------------------------------------------------------
> 
> (Updated June 27, 2018, 3:57 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio 
> Pena.
> 
> 
> Bugs: sentry-2280
>     https://issues.apache.org/jira/browse/sentry-2280
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The required fields in TSentryHmsEventNotification and 
> TSentryHmsEventNotificationResponse are not filled, which caused the request 
> from client to sever to be null. And the response from server to client is 
> null. Change the data structure to make owner info optional, and fill other 
> required fields properly
> 
> 
> Diffs
> -----
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHmsEvent.java
>  42be3c3 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
>  f7d1b07 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/metastore/TestSentrySyncHMSNotificationsPostEventListener.java
>  8e79cac 
>   
> sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryHmsEventNotification.java
>  75b2799 
>   
> sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift
>  56aedcb 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  3927150 
> 
> 
> Diff: https://reviews.apache.org/r/67749/diff/3/
> 
> 
> Testing
> -------
> 
> Manually trace into code and verfied that now the server receives request and 
> client receiveds response correctly
> 
> 
> Thanks,
> 
> Na Li
> 
>

Reply via email to