> On Dec. 8, 2016, 3:55 p.m., Aihua Xu wrote: > > service/src/java/org/apache/hive/service/auth/ldap/GroupFilterFactory.java, > > line 145 > > <https://reviews.apache.org/r/53204/diff/3/?file=1579541#file1579541line145> > > > > You may need to change message since it's expected that the user is not > > in some groups. Probably change to "Cannot match user ... and group ..." > > since "Failed to" seems to be an error. > > Illya Yalovyy wrote: > I will update the message.
Usually it should just return true or false. If it fails with exception then something is wrong. That was reflected in the message. I noticed that I'm hiding the exception, which is a very bad practice. Will fix it as well. May be even WARN log message with exception details is required here. What you think? Again it should not happen usually, if it does - something wrong. - 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 > >
