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



src/java/main/org/apache/zookeeper/ClientCnxn.java
<https://reviews.apache.org/r/1959/#comment4446>

    Could this condition be simplified? For example instead of 
    clientCnxn.zooKeeperSaslClient.getSaslState() == 
ZooKeeperSaslClient.SaslState.FAILED
    
    one could call clientCnxn.zooKeeperSaslClient.hasFailed()


- Thomas


On 2011-09-19 20:08:50, Eugene Koontz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1959/
> -----------------------------------------------------------
> 
> (Updated 2011-09-19 20:08:50)
> 
> 
> 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 723efa1 
>   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
> 
>

Reply via email to