Ivan Kelly commented on ZOOKEEPER-823:

A few comments on the current patch.

There's a lot of instanceof used with exceptions. Really you should give each 
exception it's own catch, and then put the common code in a finally clause.

This method has a lot of magic numbers being used. These should be defined as 
consts somewhere and used as such.

Theres a TODO about making SendThread an implementation of Runnable. This 
change should be fairly simple, so why not do it before submitting? 

Are all these try/catches necessary? A single try catch around the whole lot 
would do. If you need to see where it happened, you can look at the stacktrace.

One other thing to keep in mind is that this patch with conflict with 702, so 
whichever get submitted second will have to be careful not to break the other's 

> update ZooKeeper java client to optionally use Netty for connections
> --------------------------------------------------------------------
>                 Key: ZOOKEEPER-823
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-823
>             Project: Zookeeper
>          Issue Type: New Feature
>          Components: java client
>            Reporter: Patrick Hunt
>            Assignee: Patrick Hunt
>             Fix For: 3.4.0
>         Attachments: ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, 
> ZOOKEEPER-823.patch, ZOOKEEPER-823.patch, ZOOKEEPER-823.patch
> This jira will port the client side connection code to use netty rather than 
> direct nio.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

Reply via email to