----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53204/#review157121 -----------------------------------------------------------
service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 90) <https://reviews.apache.org/r/53204/#comment227538> This is a DEBUG level logging. It will not happen in production environment. The purpose of this log message is to help troubleshoot problems. Without name and group it makes little sense. I would prefer to keep it as is. service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 124) <https://reviews.apache.org/r/53204/#comment227548> I will remove sensitive information from all error messages and exceptions. Thank you for suggesting it. I will move them to a debug logs instead. It will keep security and maintainability at high level. service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java (line 138) <https://reviews.apache.org/r/53204/#comment227540> This is a DEBUG level logging. It will not happen in production environment. The purpose of this log message is to help troubleshoot problems. Without name and group it makes little sense. I would prefer to keep it as is. service/src/test/org/apache/hive/service/auth/TestLdapAuthenticationProviderImpl.java (line 261) <https://reviews.apache.org/r/53204/#comment227542> I don't think it is the best practice to add javadoc to unit test methods. Usually meaningful method names are used instead. If you look at hive code base, it is not very common as well. Only few unit tests have javadoc, some of them already obsolete and misleading. I would prefer keep it as is. service/src/test/org/apache/hive/service/auth/ldap/TestGroupFilter.java (line 32) <https://reviews.apache.org/r/53204/#comment227545> Hamcrest module is a part of JUnit. This module is designed to be used in conjunction with junit/mockito. I see no reason to break this best practice. - Illya Yalovyy 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 > >
