hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Naveen Gangam <ngan...@cloudera.com>
Subject Re: Review Request 53204: HIVE-15076 Improve scalability of LDAP authentication provider group filter
Date Mon, 28 Nov 2016 17:09:10 GMT

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




service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 90)
<https://reviews.apache.org/r/53204/#comment227431>

    Its not a good idea to log the name of the group the user belongs to whether it succeeds
or fails. So perhaps just log the fact that authentication succeeded based on group filter?



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 124)
<https://reviews.apache.org/r/53204/#comment227433>

    Remove group name from the log record.



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 130)
<https://reviews.apache.org/r/53204/#comment227436>

    anonymize the log record, should have no indication of the configuration



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 138)
<https://reviews.apache.org/r/53204/#comment227432>

    Same as above. Remove the group name from the log record.



service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 143)
<https://reviews.apache.org/r/53204/#comment227437>

    same here



service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java (line
261)
<https://reviews.apache.org/r/53204/#comment227440>

    Please add javadoc comments as to what this test is supposed to test and how its being
accomplished.



service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java (line
282)
<https://reviews.apache.org/r/53204/#comment227441>

    add javadoc comments for the test



service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java (line
299)
<https://reviews.apache.org/r/53204/#comment227442>

    Add javadoc comments for the test



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 32)
<https://reviews.apache.org/r/53204/#comment227445>

    can the usage of hamcrest APIs be replaced by JDK regex ?



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 108)
<https://reviews.apache.org/r/53204/#comment227446>

    Add javadocs



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 119)
<https://reviews.apache.org/r/53204/#comment227447>

    Add javadoc comments for the test



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 134)
<https://reviews.apache.org/r/53204/#comment227449>

    Add javadoc comments for the test



service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 149)
<https://reviews.apache.org/r/53204/#comment227450>

    Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 212)
<https://reviews.apache.org/r/53204/#comment227452>

    Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 225)
<https://reviews.apache.org/r/53204/#comment227453>

    Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 235)
<https://reviews.apache.org/r/53204/#comment227454>

    Add javadoc comments for the test



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 246)
<https://reviews.apache.org/r/53204/#comment227455>

    Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 264)
<https://reviews.apache.org/r/53204/#comment227456>

    Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestLdapSearch.java (line 279)
<https://reviews.apache.org/r/53204/#comment227457>

    Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java (line 81)
<https://reviews.apache.org/r/53204/#comment227458>

    Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java (line 90)
<https://reviews.apache.org/r/53204/#comment227459>

    Add javadoc comments for the test.



service/src/test/org/apache/hive/service/auth/ldap/TestQueryFactory.java (line 96)
<https://reviews.apache.org/r/53204/#comment227460>

    Add javadoc comments for the test.


- Naveen Gangam


On Oct. 28, 2016, 12:59 p.m., Illya Yalovyy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53204/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2016, 12:59 p.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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message