> On 2011-09-19 20:31:15, Thomas Koch wrote: > > src/java/main/org/apache/zookeeper/ClientCnxn.java, line 563 > > <https://reviews.apache.org/r/1959/diff/1/?file=41755#file41755line563> > > > > Could this condition be simplified? For example instead of > > clientCnxn.zooKeeperSaslClient.getSaslState() == > > ZooKeeperSaslClient.SaslState.FAILED > > > > one could call clientCnxn.zooKeeperSaslClient.hasFailed()
Thanks Thomas; uploaded new patch with this change. - Eugene ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1959/#review1967 ----------------------------------------------------------- On 2011-09-20 23:59:16, Eugene Koontz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1959/ > ----------------------------------------------------------- > > (Updated 2011-09-20 23:59:16) > > > Review request for zookeeper. > > > Summary > ------- > > There are 3 places where ClientCnxn should queue a AuthFailed event if client > fails to authenticate. Without sending this event, clients may be stuck > watching for a SaslAuthenticated event that will never come (since the client > failed to authenticate). > > This patch fixes SaslAuthFailTest.testBadSaslAuthNotifiesWatch() to test for > the AuthFailed event : previously, the test was incorrectly not testing for > this event. > > It also removes the testBadSaslAuthNotifiesWatch() method from the > SaslAuthTest class : this method belongs in SaslAuthFailTest, not > SaslAuthTest. The former tests unsuccessful SASL authentication; the latter, > successful SASL authentication. > > > Diffs > ----- > > src/java/main/org/apache/zookeeper/ClientCnxn.java db15348 > src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 43382c8 > src/java/test/org/apache/zookeeper/test/SaslAuthFailTest.java 8de7c2a > src/java/test/org/apache/zookeeper/test/SaslAuthTest.java fd20346 > > Diff: https://reviews.apache.org/r/1959/diff > > > Testing > ------- > > All unit tests pass. Also tested with an HBase cluster with an hbase shell > running as an unauthenticated Zookeeper client. As expected, hbase shell > could not access cluster, but, as expected, did not hang. > > > Thanks, > > Eugene > >
