> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java, line 
> > 138
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579544#file1579544line138>
> >
> >     Looks like we won't handle NPE so NPE may cause some problems. 
> >     
> >     If userMembershipAttr is null, will we still check userMememberOfGroup 
> > or not? If not, maybe we should handle such exception here. How about 
> > groupMembershipAttr above? Seems we will have such issue as well.
> 
> Illya Yalovyy wrote:
>     I think it should not happen, but I'll double check.

The filter will not be created for a case when 
'hive.server2.authentication.ldap.userMembershipKey' is not set (NULL). It 
means we don't have to handle null in this code. If this NPE happens, it means 
there is a bug in the code.


> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote:
> > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, 
> > line 115
> > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line115>
> >
> >     This should be info level which will be consistent with 
> > GroupMembershipKeyFilter class.
> 
> Illya Yalovyy wrote:
>     Ok. I'll generate 2 log entries then: 1. INFO without group information; 
> 2. DEBUG with full information. 
>     
>     Does it make sense?
>     
>     See Naveen's comments for more details.

Actually this is a bit different. I'll change it to INFO.


- Illya


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


On Dec. 8, 2016, 12:45 a.m., Illya Yalovyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2016, 12:45 a.m.)
> 
> 
> Review request for hive, Aihua Xu, Ashutosh Chauhan, Chaoyu Tang, and Szehon 
> Ho.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15076 Improve scalability of LDAP authentication provider group filter
> 
> https://issues.apache.org/jira/browse/HIVE-15076
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5ea9751 
>   service/src/java/org/apache/hive/service/auth/ldap/DirSearch.java 33b6088 
>   service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java 
> 152c4b2 
>   service/src/java/org/apache/hive/service/auth/ldap/LdapSearch.java 65076ea 
>   service/src/java/org/apache/hive/service/auth/ldap/Query.java b8bf938 
>   service/src/java/org/apache/hive/service/auth/ldap/QueryFactory.java 
> e9172d3 
>   
> service/src/test/org/apache/hive/service/auth/TestLdapAtnProviderWithMiniDS.java
>  cd62935 
>   
> service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java
>  4fad755 
>   
> service/src/test/org/apache/hive/service/auth/ldap/LdapAuthenticationTestCase.java
>  acde8c1 
>   service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java 
> 0cc2ead 
>   service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java 
> 499b624 
>   service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java 
> 3054e33 
>   service/src/test/resources/ldap/ad.example.com.ldif PRE-CREATION 
>   service/src/test/resources/ldap/microsoft.schema.ldif PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53204/diff/
> 
> 
> Testing
> -------
> 
> Build succeeded.
> 
> Test results:
> 
> Tests run: 149, Failures: 0, Errors: 0, Skipped: 0
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 03:14 min
> [INFO] Finished at: 2016-10-26T13:53:15-07:00
> [INFO] Final Memory: 36M/1091M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Illya Yalovyy
> 
>

Reply via email to