> On July 18, 2017, 8:49 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Line 83 (original), 85 (patched)
> > <https://reviews.apache.org/r/60883/diff/3/?file=1778711#file1778711line86>
> >
> >     updating authServerName(arguemnt) provided is confusing. why not update 
> > the member variable directly?
> 
> Na Li wrote:
>     I tried to update the member variable after it is assigned input value. I 
> have error since the member variable is final. So I keep it unchanged. Are 
> you OK with that?

variable was made final as it was not updated. If you have to update it feel 
free to do so by making it non final.


- kalyan kumar


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


On July 21, 2017, 2:30 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60883/
> -----------------------------------------------------------
> 
> (Updated July 21, 2017, 2:30 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Sergio Pena, and Vamsee 
> Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> There are several problems:
> 1) HMSFollower should read sentry-site.xml to get the authen server name, not 
> from hive-site.xml through hiveConf. The input configuration for HMSFollower 
> constructor is from sentry-site.xm. We should use that configuration to get 
> server name.
> 2) There are two variable names that could hold the value of the server. 
> "hive.sentry.server"is deprecated and is in 
> HiveAuthzConf.AuthzConfVars.AUTHZ_SERVER_NAME_DEPRECATED. The new name is
> sentry.hive.server in HiveAuthzConf.AuthzConfVars.AUTHZ_SERVER_NAME. We 
> should check the sentry.hive.server first. If it is not set, check the 
> deprecated hive.sentry.server to be backward compatible. If it is still not 
> set, use default value
> 
> 
> Diffs
> -----
> 
>   
> sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java
>  e1312bf 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzConf.java
>  dccbbb6 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  547a61f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
>  3d67401 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
>  fdf52bf 
> 
> 
> Diff: https://reviews.apache.org/r/60883/diff/5/
> 
> 
> Testing
> -------
> 
> create integration tests. reproduced the issue and verfied the fix
> 
> 
> Thanks,
> 
> Na Li
> 
>

Reply via email to