[GitHub] [zookeeper] Randgalt commented on a change in pull request #1095: ZOOKEEPER-837: Eliminate cycle dependency between ClientCnxn and ZooKeeper

[GitBox <gi...@apache.org>]

Randgalt commented on a change in pull request #1095: ZOOKEEPER-837: Eliminate cycle dependency between ClientCnxn and ZooKeeper
URL: https://github.com/apache/zookeeper/pull/1095#discussion_r361195558
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##########
 @@ -352,35 +354,29 @@ public String toString() {
      * established until needed. The start() instance method must be called
      * subsequent to construction.
      *
-     * @param chrootPath - the chroot of this client. Should be removed from this Class in ZOOKEEPER-838
-     * @param hostProvider
-     *                the list of ZooKeeper servers to connect to
-     * @param sessionTimeout
-     *                the timeout for connections.
-     * @param zooKeeper
-     *                the zookeeper object that this connection is related to.
-     * @param watcher watcher for this connection
-     * @param clientCnxnSocket
-     *                the socket implementation used (e.g. NIO/Netty)
-     * @param canBeReadOnly
-     *                whether the connection is allowed to go to read-only
-     *                mode in case of partitioning
-     * @throws IOException
+     * @param chrootPath the chroot of this client. Should be removed from this Class in ZOOKEEPER-838
+     * @param hostProvider the list of ZooKeeper servers to connect to
+     * @param sessionTimeout the timeout for connections.
+     * @param clientConfig the client configuration.
+     * @param defaultWatcher default watcher for this connection
+     * @param clientCnxnSocket the socket implementation used (e.g. NIO/Netty)
+     * @param canBeReadOnly whether the connection is allowed to go to read-only mode in case of partitioning
      */
     public ClientCnxn(
         String chrootPath,
         HostProvider hostProvider,
         int sessionTimeout,
-        ZooKeeper zooKeeper,
-        ClientWatchManager watcher,
+        ZKClientConfig clientConfig,
 
 Review comment:
   On first glance it doesn't look like it conflicts with Curator. But, maybe @TisonKun can test with Curator locally. As an aside, I wonder if we shouldn't include some Curator test in ZooKeeper in the future - i.e. validate that new changes don't break Curator. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services