You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/03/14 21:59:55 UTC

[GitHub] [zookeeper] sankalpbhatia opened a new pull request #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

sankalpbhatia opened a new pull request #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285
 
 
   The current zkCli uses system properties to set confidential information like keystore location, password etc. This is not secure as these properties need to be passed on the command line as "-D"  arguments. 
   
   Currently, there is no way to create a ZookeeperAdmin does not have a constructor which takes both canBeReadOnly and ZKClientConfig as parameters.  I am introducing a new constructor in ZookeeperAdmin which takes an additional ZKClientConfig parameter. This ZKClientConfig is created by an optional command line argument ``client-configuration``. If no argument is passed, a ZookeeperAdmin object with null client config is created, just like before. 

----------------------------------------------------------------
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

[GitHub] [zookeeper] sankalpbhatia commented on issue #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
sankalpbhatia commented on issue #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#issuecomment-600373906
 
 
   Thanks @maoling and @eolivelli. I updated the PR.

----------------------------------------------------------------
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

[GitHub] [zookeeper] sankalpbhatia commented on a change in pull request #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
sankalpbhatia commented on a change in pull request #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#discussion_r395171220
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/admin/ZooKeeperAdmin.java
 ##########
 @@ -124,6 +124,52 @@ public ZooKeeperAdmin(
         super(connectString, sessionTimeout, watcher, conf);
     }
 
+    /**
+     * Create a ZooKeeperAdmin object which is used to perform dynamic reconfiguration
+     * operations.
+     *
+     * @param connectString
+     *            comma separated host:port pairs, each corresponding to a zk
+     *            server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" If
+     *            the optional chroot suffix is used the example would look
+     *            like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
+     *            where the client would be rooted at "/app/a" and all paths
+     *            would be relative to this root - ie getting/setting/etc...
+     *            "/foo/bar" would result in operations being run on
+     *            "/app/a/foo/bar" (from the server perspective).
+     * @param sessionTimeout
+     *            session timeout in milliseconds
+     * @param watcher
+     *            a watcher object which will be notified of state changes, may
+     *            also be notified for node events
+     * @param canBeReadOnly
+     *            whether the created client is allowed to go to
+     *            read-only mode in case of partitioning. Read-only mode
+     *            basically means that if the client can't find any majority
+     *            servers but there's partitioned server it could reach, it
+     *            connects to one in read-only mode, i.e. read requests are
+     *            allowed while write requests are not. It continues seeking for
+     *            majority in the background.
+     * @param conf
+     *            passing this conf object gives each client the flexibility of
+     *            configuring properties differently compared to other instances
+     *
+     * @throws IOException
+     *             in cases of network failure
+     * @throws IllegalArgumentException
+     *             if an invalid chroot path is specified
+     *
+     * @see ZooKeeper#ZooKeeper(String, int, Watcher, boolean, ZKClientConfig)
+     */
 
 Review comment:
   Thanks. I added a commit to change the version

----------------------------------------------------------------
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

[GitHub] [zookeeper] maoling commented on a change in pull request #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
maoling commented on a change in pull request #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#discussion_r392673132
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/admin/ZooKeeperAdmin.java
 ##########
 @@ -124,6 +124,52 @@ public ZooKeeperAdmin(
         super(connectString, sessionTimeout, watcher, conf);
     }
 
+    /**
+     * Create a ZooKeeperAdmin object which is used to perform dynamic reconfiguration
+     * operations.
+     *
+     * @param connectString
+     *            comma separated host:port pairs, each corresponding to a zk
+     *            server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" If
+     *            the optional chroot suffix is used the example would look
+     *            like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
+     *            where the client would be rooted at "/app/a" and all paths
+     *            would be relative to this root - ie getting/setting/etc...
+     *            "/foo/bar" would result in operations being run on
+     *            "/app/a/foo/bar" (from the server perspective).
+     * @param sessionTimeout
+     *            session timeout in milliseconds
+     * @param watcher
+     *            a watcher object which will be notified of state changes, may
+     *            also be notified for node events
+     * @param canBeReadOnly
+     *            whether the created client is allowed to go to
+     *            read-only mode in case of partitioning. Read-only mode
+     *            basically means that if the client can't find any majority
+     *            servers but there's partitioned server it could reach, it
+     *            connects to one in read-only mode, i.e. read requests are
+     *            allowed while write requests are not. It continues seeking for
+     *            majority in the background.
+     * @param conf
+     *            passing this conf object gives each client the flexibility of
+     *            configuring properties differently compared to other instances
+     *
+     * @throws IOException
+     *             in cases of network failure
+     * @throws IllegalArgumentException
+     *             if an invalid chroot path is specified
+     *
+     * @see ZooKeeper#ZooKeeper(String, int, Watcher, boolean, ZKClientConfig)
+     */
 
 Review comment:
   add this `@since 3.7.0` is better?

----------------------------------------------------------------
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

[GitHub] [zookeeper] maoling commented on a change in pull request #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
maoling commented on a change in pull request #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#discussion_r392756824
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/admin/ZooKeeperAdmin.java
 ##########
 @@ -124,6 +124,52 @@ public ZooKeeperAdmin(
         super(connectString, sessionTimeout, watcher, conf);
     }
 
+    /**
+     * Create a ZooKeeperAdmin object which is used to perform dynamic reconfiguration
+     * operations.
+     *
+     * @param connectString
+     *            comma separated host:port pairs, each corresponding to a zk
+     *            server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" If
+     *            the optional chroot suffix is used the example would look
+     *            like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
+     *            where the client would be rooted at "/app/a" and all paths
+     *            would be relative to this root - ie getting/setting/etc...
+     *            "/foo/bar" would result in operations being run on
+     *            "/app/a/foo/bar" (from the server perspective).
+     * @param sessionTimeout
+     *            session timeout in milliseconds
+     * @param watcher
+     *            a watcher object which will be notified of state changes, may
+     *            also be notified for node events
+     * @param canBeReadOnly
+     *            whether the created client is allowed to go to
+     *            read-only mode in case of partitioning. Read-only mode
+     *            basically means that if the client can't find any majority
+     *            servers but there's partitioned server it could reach, it
+     *            connects to one in read-only mode, i.e. read requests are
+     *            allowed while write requests are not. It continues seeking for
+     *            majority in the background.
+     * @param conf
+     *            passing this conf object gives each client the flexibility of
+     *            configuring properties differently compared to other instances
+     *
+     * @throws IOException
+     *             in cases of network failure
+     * @throws IllegalArgumentException
+     *             if an invalid chroot path is specified
+     *
+     * @see ZooKeeper#ZooKeeper(String, int, Watcher, boolean, ZKClientConfig)
+     */
 
 Review comment:
   It could not be better you can also update the CLI doc and add a usage example https://github.com/apache/zookeeper/blob/master/zookeeper-docs/src/main/resources/markdown/zookeeperCLI.md#help

----------------------------------------------------------------
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

[GitHub] [zookeeper] eolivelli commented on issue #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#issuecomment-601133062
 
 
   @nkalmar this is a good candidate for 3.6.1
   
   please take a look

----------------------------------------------------------------
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

[GitHub] [zookeeper] eolivelli closed pull request #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
eolivelli closed pull request #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285
 
 
   

----------------------------------------------------------------
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

[GitHub] [zookeeper] nkalmar commented on a change in pull request #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
nkalmar commented on a change in pull request #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#discussion_r394978753
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/admin/ZooKeeperAdmin.java
 ##########
 @@ -124,6 +124,52 @@ public ZooKeeperAdmin(
         super(connectString, sessionTimeout, watcher, conf);
     }
 
+    /**
+     * Create a ZooKeeperAdmin object which is used to perform dynamic reconfiguration
+     * operations.
+     *
+     * @param connectString
+     *            comma separated host:port pairs, each corresponding to a zk
+     *            server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" If
+     *            the optional chroot suffix is used the example would look
+     *            like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
+     *            where the client would be rooted at "/app/a" and all paths
+     *            would be relative to this root - ie getting/setting/etc...
+     *            "/foo/bar" would result in operations being run on
+     *            "/app/a/foo/bar" (from the server perspective).
+     * @param sessionTimeout
+     *            session timeout in milliseconds
+     * @param watcher
+     *            a watcher object which will be notified of state changes, may
+     *            also be notified for node events
+     * @param canBeReadOnly
+     *            whether the created client is allowed to go to
+     *            read-only mode in case of partitioning. Read-only mode
+     *            basically means that if the client can't find any majority
+     *            servers but there's partitioned server it could reach, it
+     *            connects to one in read-only mode, i.e. read requests are
+     *            allowed while write requests are not. It continues seeking for
+     *            majority in the background.
+     * @param conf
+     *            passing this conf object gives each client the flexibility of
+     *            configuring properties differently compared to other instances
+     *
+     * @throws IOException
+     *             in cases of network failure
+     * @throws IllegalArgumentException
+     *             if an invalid chroot path is specified
+     *
+     * @see ZooKeeper#ZooKeeper(String, int, Watcher, boolean, ZKClientConfig)
+     */
 
 Review comment:
   Okay, thanks all! 

----------------------------------------------------------------
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

[GitHub] [zookeeper] sankalpbhatia commented on issue #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
sankalpbhatia commented on issue #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#issuecomment-602861683
 
 
   @eolivelli @nkalmar please take a look

----------------------------------------------------------------
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

[GitHub] [zookeeper] eolivelli commented on a change in pull request #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#discussion_r394974422
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/admin/ZooKeeperAdmin.java
 ##########
 @@ -124,6 +124,52 @@ public ZooKeeperAdmin(
         super(connectString, sessionTimeout, watcher, conf);
     }
 
+    /**
+     * Create a ZooKeeperAdmin object which is used to perform dynamic reconfiguration
+     * operations.
+     *
+     * @param connectString
+     *            comma separated host:port pairs, each corresponding to a zk
+     *            server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" If
+     *            the optional chroot suffix is used the example would look
+     *            like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
+     *            where the client would be rooted at "/app/a" and all paths
+     *            would be relative to this root - ie getting/setting/etc...
+     *            "/foo/bar" would result in operations being run on
+     *            "/app/a/foo/bar" (from the server perspective).
+     * @param sessionTimeout
+     *            session timeout in milliseconds
+     * @param watcher
+     *            a watcher object which will be notified of state changes, may
+     *            also be notified for node events
+     * @param canBeReadOnly
+     *            whether the created client is allowed to go to
+     *            read-only mode in case of partitioning. Read-only mode
+     *            basically means that if the client can't find any majority
+     *            servers but there's partitioned server it could reach, it
+     *            connects to one in read-only mode, i.e. read requests are
+     *            allowed while write requests are not. It continues seeking for
+     *            majority in the background.
+     * @param conf
+     *            passing this conf object gives each client the flexibility of
+     *            configuring properties differently compared to other instances
+     *
+     * @throws IOException
+     *             in cases of network failure
+     * @throws IllegalArgumentException
+     *             if an invalid chroot path is specified
+     *
+     * @see ZooKeeper#ZooKeeper(String, int, Watcher, boolean, ZKClientConfig)
+     */
 
 Review comment:
   I have just added the 3.6.1 tag, initially this pr was targeted for 3.7.
   
   can you please @sangitanalkar change to 3.6.1 ?

----------------------------------------------------------------
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

[GitHub] [zookeeper] nkalmar commented on a change in pull request #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
nkalmar commented on a change in pull request #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#discussion_r394972580
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/admin/ZooKeeperAdmin.java
 ##########
 @@ -124,6 +124,52 @@ public ZooKeeperAdmin(
         super(connectString, sessionTimeout, watcher, conf);
     }
 
+    /**
+     * Create a ZooKeeperAdmin object which is used to perform dynamic reconfiguration
+     * operations.
+     *
+     * @param connectString
+     *            comma separated host:port pairs, each corresponding to a zk
+     *            server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" If
+     *            the optional chroot suffix is used the example would look
+     *            like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
+     *            where the client would be rooted at "/app/a" and all paths
+     *            would be relative to this root - ie getting/setting/etc...
+     *            "/foo/bar" would result in operations being run on
+     *            "/app/a/foo/bar" (from the server perspective).
+     * @param sessionTimeout
+     *            session timeout in milliseconds
+     * @param watcher
+     *            a watcher object which will be notified of state changes, may
+     *            also be notified for node events
+     * @param canBeReadOnly
+     *            whether the created client is allowed to go to
+     *            read-only mode in case of partitioning. Read-only mode
+     *            basically means that if the client can't find any majority
+     *            servers but there's partitioned server it could reach, it
+     *            connects to one in read-only mode, i.e. read requests are
+     *            allowed while write requests are not. It continues seeking for
+     *            majority in the background.
+     * @param conf
+     *            passing this conf object gives each client the flexibility of
+     *            configuring properties differently compared to other instances
+     *
+     * @throws IOException
+     *             in cases of network failure
+     * @throws IllegalArgumentException
+     *             if an invalid chroot path is specified
+     *
+     * @see ZooKeeper#ZooKeeper(String, int, Watcher, boolean, ZKClientConfig)
+     */
 
 Review comment:
   If we want this in 3.6.1 why not just write "@since 3.6.1" ?

----------------------------------------------------------------
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

[GitHub] [zookeeper] nkalmar commented on a change in pull request #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
nkalmar commented on a change in pull request #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#discussion_r394972580
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/admin/ZooKeeperAdmin.java
 ##########
 @@ -124,6 +124,52 @@ public ZooKeeperAdmin(
         super(connectString, sessionTimeout, watcher, conf);
     }
 
+    /**
+     * Create a ZooKeeperAdmin object which is used to perform dynamic reconfiguration
+     * operations.
+     *
+     * @param connectString
+     *            comma separated host:port pairs, each corresponding to a zk
+     *            server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" If
+     *            the optional chroot suffix is used the example would look
+     *            like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
+     *            where the client would be rooted at "/app/a" and all paths
+     *            would be relative to this root - ie getting/setting/etc...
+     *            "/foo/bar" would result in operations being run on
+     *            "/app/a/foo/bar" (from the server perspective).
+     * @param sessionTimeout
+     *            session timeout in milliseconds
+     * @param watcher
+     *            a watcher object which will be notified of state changes, may
+     *            also be notified for node events
+     * @param canBeReadOnly
+     *            whether the created client is allowed to go to
+     *            read-only mode in case of partitioning. Read-only mode
+     *            basically means that if the client can't find any majority
+     *            servers but there's partitioned server it could reach, it
+     *            connects to one in read-only mode, i.e. read requests are
+     *            allowed while write requests are not. It continues seeking for
+     *            majority in the background.
+     * @param conf
+     *            passing this conf object gives each client the flexibility of
+     *            configuring properties differently compared to other instances
+     *
+     * @throws IOException
+     *             in cases of network failure
+     * @throws IllegalArgumentException
+     *             if an invalid chroot path is specified
+     *
+     * @see ZooKeeper#ZooKeeper(String, int, Watcher, boolean, ZKClientConfig)
+     */
 
 Review comment:
   If we want this in 3.6.1 why not just write `@since 3.6.1` ?

----------------------------------------------------------------
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

[GitHub] [zookeeper] sankalpbhatia commented on a change in pull request #1285: ZOOKEEPER-3689: zkCli/ZooKeeperMain relies on system properties for TLS config

Posted by GitBox <gi...@apache.org>.
sankalpbhatia commented on a change in pull request #1285: ZOOKEEPER-3689:  zkCli/ZooKeeperMain relies on system properties for TLS config
URL: https://github.com/apache/zookeeper/pull/1285#discussion_r394043780
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/admin/ZooKeeperAdmin.java
 ##########
 @@ -124,6 +124,52 @@ public ZooKeeperAdmin(
         super(connectString, sessionTimeout, watcher, conf);
     }
 
+    /**
+     * Create a ZooKeeperAdmin object which is used to perform dynamic reconfiguration
+     * operations.
+     *
+     * @param connectString
+     *            comma separated host:port pairs, each corresponding to a zk
+     *            server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" If
+     *            the optional chroot suffix is used the example would look
+     *            like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
+     *            where the client would be rooted at "/app/a" and all paths
+     *            would be relative to this root - ie getting/setting/etc...
+     *            "/foo/bar" would result in operations being run on
+     *            "/app/a/foo/bar" (from the server perspective).
+     * @param sessionTimeout
+     *            session timeout in milliseconds
+     * @param watcher
+     *            a watcher object which will be notified of state changes, may
+     *            also be notified for node events
+     * @param canBeReadOnly
+     *            whether the created client is allowed to go to
+     *            read-only mode in case of partitioning. Read-only mode
+     *            basically means that if the client can't find any majority
+     *            servers but there's partitioned server it could reach, it
+     *            connects to one in read-only mode, i.e. read requests are
+     *            allowed while write requests are not. It continues seeking for
+     *            majority in the background.
+     * @param conf
+     *            passing this conf object gives each client the flexibility of
+     *            configuring properties differently compared to other instances
+     *
+     * @throws IOException
+     *             in cases of network failure
+     * @throws IllegalArgumentException
+     *             if an invalid chroot path is specified
+     *
+     * @see ZooKeeper#ZooKeeper(String, int, Watcher, boolean, ZKClientConfig)
+     */
 
 Review comment:
   Thanks @maoling for the review. I have addressed both your comments in the next commit

----------------------------------------------------------------
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