[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314588811
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperSaslClient.java
 ##
 @@ -77,7 +77,7 @@
  */
 @Deprecated
 public static boolean isEnabled() {
-return 
Boolean.valueOf(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
+return 
Boolean.getBoolean(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
 
 Review comment:
   Should never do refactor in this pr then.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314588811
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperSaslClient.java
 ##
 @@ -77,7 +77,7 @@
  */
 @Deprecated
 public static boolean isEnabled() {
-return 
Boolean.valueOf(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
+return 
Boolean.getBoolean(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
 
 Review comment:
   Should never do refactor then.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314588414
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperSaslClient.java
 ##
 @@ -77,7 +77,7 @@
  */
 @Deprecated
 public static boolean isEnabled() {
-return 
Boolean.valueOf(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
+return 
Boolean.getBoolean(System.getProperty(ZKClientConfig.ENABLE_CLIENT_SASL_KEY, 
ZKClientConfig.ENABLE_CLIENT_SASL_DEFAULT));
 
 Review comment:
   Reverted as `Boolean.parseBoolean`. It should never be 
`Boolean.getBoolean("true"|"false")` which would nearly always returns false. 


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314588183
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java
 ##
 @@ -99,15 +99,16 @@ public FileTxnSnapLog(File dataDir, File snapDir) throws 
IOException {
 
 // by default create snap/log dirs, but otherwise complain instead
 // See ZOOKEEPER-1161 for more details
-boolean enableAutocreate = 
Boolean.valueOf(System.getProperty(ZOOKEEPER_DATADIR_AUTOCREATE, 
ZOOKEEPER_DATADIR_AUTOCREATE_DEFAULT));
+boolean enableAutocreate = Boolean.getBoolean(
 
 Review comment:
   This is a buggy change. Revert.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314581891
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -1268,11 +1256,7 @@ public void run() {
 } else if (e instanceof SocketException) {
 LOG.info("Socket error occurred: {}: {}", 
serverAddress, e.getMessage());
 } else {
-LOG.warn("Session 0x{} for server {}, unexpected 
error{}",
-Long.toHexString(getSessionId()),
-serverAddress,
-RETRY_CONN_MSG,
-e);
+LOG.warn("Session 0x{} for server {}, unexpected 
error{}", Long.toHexString(getSessionId()), serverAddress, RETRY_CONN_MSG, e);
 
 Review comment:
   In fact I locally turn on `LineLength` and see that there is about 200 files 
break the rules, a pass to take care of it is ok for this pr. But not all of 
them have a reasonable break down strategy to apply on. Some of them are just 
because too much indent level so break line length rule.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314580611
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -1268,11 +1256,7 @@ public void run() {
 } else if (e instanceof SocketException) {
 LOG.info("Socket error occurred: {}: {}", 
serverAddress, e.getMessage());
 } else {
-LOG.warn("Session 0x{} for server {}, unexpected 
error{}",
-Long.toHexString(getSessionId()),
-serverAddress,
-RETRY_CONN_MSG,
-e);
+LOG.warn("Session 0x{} for server {}, unexpected 
error{}", Long.toHexString(getSessionId()), serverAddress, RETRY_CONN_MSG, e);
 
 Review comment:
   It is possible that we enable `LineLength` rule on zookeeper-server that 
forbid every line exceed a preconfigure value. See ZOOKEEPER-3508 for it.
   
   We can tell an auto-formatter to break a line but it would introduce awful 
breaks as Enrico request to change.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314581508
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -925,15 +926,11 @@ else if (serverPath.length() > chrootPath.length())
  */
 try {
 if (packet.requestHeader.getXid() != replyHdr.getXid()) {
-packet.replyHeader.setErr(
-KeeperException.Code.CONNECTIONLOSS.intValue());
-throw new IOException("Xid out of order. Got Xid "
-+ replyHdr.getXid() + " with err " +
-+ replyHdr.getErr() +
-" expected Xid "
-+ packet.requestHeader.getXid()
-+ " for a packet with details: "
-+ packet );
+
packet.replyHeader.setErr(KeeperException.Code.CONNECTIONLOSS.intValue());
+throw new IOException("Xid out of order. Got Xid " + 
replyHdr.getXid()
+  + " with err " + +replyHdr.getErr()
 
 Review comment:
   Nice catch. Done.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314581323
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/Watcher.java
 ##
 @@ -35,34 +35,33 @@
  * This interface defines the possible states an Event may represent
  */
 @InterfaceAudience.Public
-public interface Event {
+interface Event {
+
 /**
  * Enumeration of states the ZooKeeper may be at the event
  */
 @InterfaceAudience.Public
-public enum KeeperState {
+enum KeeperState {
 /** Unused, this state is never generated by the server */
-@Deprecated
-Unknown (-1),
+@Deprecated Unknown(-1),
 
 Review comment:
   Nope. Reverted.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314580886
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java
 ##
 @@ -109,45 +111,43 @@
 int ADMIN = 1 << 4;
 
 int ALL = READ | WRITE | CREATE | DELETE | ADMIN;
+
 }
 
 @InterfaceAudience.Public
 public interface Ids {
+
 /**
  * This Id represents anyone.
  */
-public final Id ANYONE_ID_UNSAFE = new Id("world", "anyone");
+Id ANYONE_ID_UNSAFE = new Id("world", "anyone");
 
 Review comment:
   Any field in an interface must be `public static final`. There is a rule 
`redundant modifier` check this.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314580742
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java
 ##
 @@ -200,9 +195,7 @@ public static void visitSubTreeDFS(ZooKeeper zk, final 
String path, boolean watc
 }
 
 @SuppressWarnings("unchecked")
-private static void visitSubTreeDFSHelper(ZooKeeper zk, final String path,
-boolean watch, StringCallback cb)
-throws KeeperException, InterruptedException {
+private static void visitSubTreeDFSHelper(ZooKeeper zk, final String path, 
boolean watch, StringCallback cb) throws KeeperException, InterruptedException {
 
 Review comment:
   This pattern can be resolve easier. Since chop down method declaration is a 
fixed pattern.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314580611
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -1268,11 +1256,7 @@ public void run() {
 } else if (e instanceof SocketException) {
 LOG.info("Socket error occurred: {}: {}", 
serverAddress, e.getMessage());
 } else {
-LOG.warn("Session 0x{} for server {}, unexpected 
error{}",
-Long.toHexString(getSessionId()),
-serverAddress,
-RETRY_CONN_MSG,
-e);
+LOG.warn("Session 0x{} for server {}, unexpected 
error{}", Long.toHexString(getSessionId()), serverAddress, RETRY_CONN_MSG, e);
 
 Review comment:
   It is possible that we enable `LineBreak` rule on zookeeper-server that 
forbid every line exceed a preconfigure value. See ZOOKEEPER-3508 for it.
   
   We can tell an auto-formatter to break a line but it would introduce awful 
breaks as Enrico request to change.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314554375
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -433,22 +417,22 @@ public WatcherSetEventPair(Set watchers, 
WatchedEvent event) {
  */
 private static String makeThreadName(String suffix) {
 String name = Thread.currentThread().getName().
-replaceAll("-EventThread", "");
+  
replaceAll("-EventThread", "");
 
 Review comment:
   There is no checkstyle rule guard this and I accidentally introduce it when 
auto-formatting...
   
   I will check these changes for now and try to find a rule to forbid this 
pattern. However, given how to break lines is nearly arbitrary by the meaning 
of the codes, it is hard to express such a rule.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314479796
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314416189
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314416189
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314412010
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperTestClient.java
 ##
 @@ -18,450 +18,435 @@
 
 package org.apache.zookeeper.test;
 
-import java.io.File;
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.WatchedEvent;
 import org.apache.zookeeper.Watcher;
-import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.Watcher.Event.EventType;
+import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.data.Stat;
-import org.apache.zookeeper.server.ServerCnxnFactory;
-import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
 
 public class ZooKeeperTestClient extends ZKTestCase implements Watcher {
-  protected String hostPort = "127.0.0.1:22801";
 
-  protected static final String dirOnZK = "/test_dir";
+protected String hostPort = "127.0.0.1:22801";
+
+protected static final String dirOnZK = "/test_dir";
+
+protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
 
-  protected String testDirOnZK = dirOnZK + "/" + Time.currentElapsedTime();
+LinkedBlockingQueue events = new 
LinkedBlockingQueue();
 
-  LinkedBlockingQueue events = new 
LinkedBlockingQueue();
+private WatchedEvent getEvent(int numTries) throws InterruptedException {
+WatchedEvent event = null;
+for (int i = 0; i < numTries; i++) {
+System.out.println("i = " + i);
+event = events.poll(10, TimeUnit.SECONDS);
+if (event != null) {
+break;
+}
+Thread.sleep(5000);
+}
+return event;
 
-  private WatchedEvent getEvent(int numTries) throws InterruptedException {
-WatchedEvent event = null;
-for (int i = 0; i < numTries; i++) {
-  System.out.println("i = " + i);
-  event = events.poll(10, TimeUnit.SECONDS);
-  if (event != null) {
-break;
-  }
-  Thread.sleep(5000);
 }
-return event;
 
-  }
+private void deleteZKDir(ZooKeeper zk, String nodeName) throws 
IOException, InterruptedException, KeeperException {
 
-  private void deleteZKDir(ZooKeeper zk, String nodeName)
-  throws IOException, InterruptedException, KeeperException {
+Stat stat = zk.exists(nodeName, false);
+if (stat == null) {
+return;
+}
 
-Stat stat = zk.exists(nodeName, false);
-if (stat == null) {
-  return;
-}
+List children1 = zk.getChildren(nodeName, false);
+List c2 = zk.getChildren(nodeName, false, stat);
 
-List children1 = zk.getChildren(nodeName, false);
-List c2 = zk.getChildren(nodeName, false, stat);
+if (!children1.equals(c2)) {
+Assert.fail("children lists from getChildren()/getChildren2() do 
not match");
+}
 
-if (!children1.equals(c2)) {
-Assert.fail("children lists from getChildren()/getChildren2() do not 
match");
-}
+if (!stat.equals(stat)) {
+Assert.fail("stats from exists()/getChildren2() do not match");
+}
 
-if (!stat.equals(stat)) {
-Assert.fail("stats from exists()/getChildren2() do not match");
+if (children1.size() == 0) {
+zk.delete(nodeName, -1);
+return;
+}
+for (String n : children1) {
+deleteZKDir(zk, n);
+}
 }
 
-if (children1.size() == 0) {
-  zk.delete(nodeName, -1);
-  return;
-}
-for (String n : children1) {
-  deleteZKDir(zk, n);
-}
-  }
-
-  private void checkRoot() throws IOException,
-  InterruptedException {
-ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
-
-try {
-  zk.create(dirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-Assert.fail("Unexpected exception code for create " + dirOnZK + ": "
-+ ke.getMessage());
-}
+private void checkRoot() throws IOException, InterruptedException {
+ZooKeeper zk = new ZooKeeper(hostPort, 1, this);
 
-try {
-  zk.create(testDirOnZK, null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-} catch (KeeperException.NodeExistsException ke) {
-// expected, sort of
-} catch (KeeperException ke) {
-   

[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314411673
 
 

 ##
 File path: 
zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java
 ##
 @@ -112,15 +109,29 @@ private PortForwarder setUp(final int omProxyPort) 
throws IOException {
 
 OM_PORT = PortAssignment.unique();
 
-String quorumCfgSection =
-"server.1=127.0.0.1:" + (PORT_QP1)
-+ ":" + (PORT_QP_LE1) + ";" +  CLIENT_PORT_QP1
-+ "\nserver.2=127.0.0.1:" + (PORT_QP2)
-+ ":" + (PORT_QP_LE2) + ";" + CLIENT_PORT_QP2
-+ "\nserver.3=127.0.0.1:"
-+ (PORT_OBS)+ ":" + (PORT_OBS_LE) + ":observer" + ";" 
+ CLIENT_PORT_OBS;
+String quorumCfgSection = "server.1=127.0.0.1:"
 
 Review comment:
   Yes. It requires another pass about line break things. I will take care of 
it.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314405124
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/ZooKeeperSaslClient.java
 ##
 @@ -442,11 +428,9 @@ public boolean clientTunneledAuthenticationInProgress() {
 }
 
 // 2. SASL authentication has succeeded or failed..
-if (!gotLastPacket) {
-// ..but still in progress, because there is a final SASL
-// message from server which must be received.
-return true;
-}
+// ..but still in progress, because there is a final SASL
+// message from server which must be received.
+return !gotLastPacket;
 
 Review comment:
   Reverting...


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314404402
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/ZookeeperBanner.java
 ##
 @@ -26,21 +26,12 @@
  */
 public class ZookeeperBanner {
 
-private static final String[] BANNER = {"",
-"  __  _   
   ",
-" |___  / | |  
   ",
-"/ /___ ___   | | __   ______   _ __ ___   _ 
__   ",
-"   / // _ \\   / _ \\  | |/ /  / _ \\  / _ \\ | '_ \\   / _ 
\\ | '__|",
-"  / /__  | (_) | | (_) | |   <  |  __/ |  __/ | |_) | |  __/ | |  
  " ,
-" /_|  \\___/   \\___/  |_|\\_\\  \\___|  \\___| | .__/   
\\___| |_|",
-"  | | 
" ,
-"  |_| 
",
-""
-};
+private static final String[] BANNER = {"", "  __  _   
   ", " |___  / | | 
", "/ /___ ___   | | __   ___
___   _ __ ___   _ __   ", "   / // _ \\   / _ \\  | |/ /  / _ \\  / _ 
\\ | '_ \\   / _ \\ | '__|", "  / /__  | (_) | | (_) | |   <  |  __/ |  __/ | 
|_) | |  __/ | |", " /_|  \\___/   \\___/  |_|\\_\\  \\___|  \\___| | 
.__/   \\___| |_|", "  | |  
   ", "  |_|
 ", ""};
 
 Review comment:
   Yes of course.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314404109
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/ServerAdminClient.java
 ##
 @@ -192,9 +192,12 @@ public static void setTraceMask(String host, int port, 
String traceMaskStr) {
 int rc = is.read(resBytes);
 ByteBuffer res = ByteBuffer.wrap(resBytes);
 long retv = res.getLong();
-System.out.println("rc=" + rc + " retv=0"
-+ Long.toOctalString(retv) + " masks=0"
-+ Long.toOctalString(traceMask));
+System.out.println("rc="
+   + rc
 
 Review comment:
   Reformatted


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314366835
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/SaslClientCallbackHandler.java
 ##
 @@ -50,36 +51,32 @@ public void handle(Callback[] callbacks) throws 
UnsupportedCallbackException {
 if (callback instanceof NameCallback) {
 NameCallback nc = (NameCallback) callback;
 nc.setName(nc.getDefaultName());
-}
-else {
+} else {
 if (callback instanceof PasswordCallback) {
-PasswordCallback pc = (PasswordCallback)callback;
+PasswordCallback pc = (PasswordCallback) callback;
 if (password != null) {
 pc.setPassword(this.password.toCharArray());
 } else {
-LOG.warn("Could not login: the {} is being asked for a 
password, but the ZooKeeper {}" +
-  " code does not currently support obtaining a 
password from the user." +
-  " Make sure that the {} is configured to use a 
ticket cache (using" +
-  " the JAAS configuration setting 
'useTicketCache=true)' and restart the {}. If" +
-  " you still get this message after that, the TGT in 
the ticket cache has expired and must" +
-  " be manually refreshed. To do so, first determine 
if you are using a password or a" +
-  " keytab. If the former, run kinit in a Unix shell 
in the environment of the user who" +
-  " is running this Zookeeper {} using the command" +
-  " 'kinit ' (where  is the name of the 
{}'s Kerberos principal)." +
-  " If the latter, do" +
-  " 'kinit -k -t  ' (where  is 
the name of the Kerberos principal, and" +
-  "  is the location of the keytab file). 
After manually refreshing your cache," +
-  " restart this {}. If you continue to see this 
message after manually refreshing" +
-  " your cache, ensure that your KDC host's clock is 
in sync with this host's clock.",
-  new Object[]{entity, entity, entity, entity, entity, 
entity, entity});
+LOG.warn("Could not login: the {} is being asked for a 
password, but the ZooKeeper {}"
+ + " code does not currently support 
obtaining a password from the user."
 
 Review comment:
   Reformatting...
   
   Align `+`s with the initial `"`


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314365190
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
 ##
 @@ -131,12 +128,7 @@ private Bootstrap configureBootstrapAllocator(Bootstrap 
bootstrap) {
 void connect(InetSocketAddress addr) throws IOException {
 firstConnect = new CountDownLatch(1);
 
-Bootstrap bootstrap = new Bootstrap()
-.group(eventLoopGroup)
-.channel(NettyUtils.nioOrEpollSocketChannel())
-.option(ChannelOption.SO_LINGER, -1)
-.option(ChannelOption.TCP_NODELAY, true)
-.handler(new ZKClientPipelineFactory(addr.getHostString(), 
addr.getPort()));
+Bootstrap bootstrap = new 
Bootstrap().group(eventLoopGroup).channel(NettyUtils.nioOrEpollSocketChannel()).option(ChannelOption.SO_LINGER,
 -1).option(ChannelOption.TCP_NODELAY, true).handler(new 
ZKClientPipelineFactory(addr.getHostString(), addr.getPort()));
 
 Review comment:
   Reverting...


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r314364487
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -232,17 +232,7 @@ public String toString() {
 
 SocketAddress local = 
sendThread.getClientCnxnSocket().getLocalSocketAddress();
 SocketAddress remote = 
sendThread.getClientCnxnSocket().getRemoteSocketAddress();
-sb
-.append("sessionid:0x").append(Long.toHexString(getSessionId()))
-.append(" local:").append(local)
-.append(" remoteserver:").append(remote)
-.append(" lastZxid:").append(lastZxid)
-.append(" xid:").append(xid)
-.append(" 
sent:").append(sendThread.getClientCnxnSocket().getSentCount())
-.append(" 
recv:").append(sendThread.getClientCnxnSocket().getRecvCount())
-.append(" queuedpkts:").append(outgoingQueue.size())
-.append(" pendingresp:").append(pendingQueue.size())
-.append(" queuedevents:").append(eventThread.waitingEvents.size());
+
sb.append("sessionid:0x").append(Long.toHexString(getSessionId())).append(" 
local:").append(local).append(" remoteserver:").append(remote).append(" 
lastZxid:").append(lastZxid).append(" xid:").append(xid).append(" 
sent:").append(sendThread.getClientCnxnSocket().getSentCount()).append(" 
recv:").append(sendThread.getClientCnxnSocket().getRecvCount()).append(" 
queuedpkts:").append(outgoingQueue.size()).append(" 
pendingresp:").append(pendingQueue.size()).append(" 
queuedevents:").append(eventThread.waitingEvents.size());
 
 Review comment:
   Reverting...


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312900302
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/HostProvider.java
 ##
 @@ -18,58 +18,60 @@
 
 package org.apache.zookeeper.client;
 
-import org.apache.yetus.audience.InterfaceAudience;
-
 import java.net.InetSocketAddress;
 import java.util.Collection;
+import org.apache.yetus.audience.InterfaceAudience;
 
 /**
  * A set of hosts a ZooKeeper client should connect to.
- * 
- * Classes implementing this interface must guarantee the following:
- * 
- * * Every call to next() returns an InetSocketAddress. So the iterator never
- * ends.
- * 
- * * The size() of a HostProvider may never be zero.
- * 
- * A HostProvider must return resolved InetSocketAddress instances on next() 
if the next address is resolvable.
+ *
+ * Classes implementing this interface must guarantee the following:
+ *
+ * 
+ * Every call to next() returns an InetSocketAddress. So the iterator 
never ends.
+ * The size() of a HostProvider may never be zero.
+ * 
+ *
+ * A HostProvider must return resolved InetSocketAddress instances on 
next() if the next address is resolvable.
  * In that case, it's up to the HostProvider, whether it returns the next 
resolvable address in the list or return
  * the next one as UnResolved.
- * 
- * Different HostProvider could be imagined:
- * 
- * * A HostProvider that loads the list of Hosts from an URL or from DNS 
- * * A HostProvider that re-resolves the InetSocketAddress after a timeout. 
- * * A HostProvider that prefers nearby hosts.
+ *
+ * ifferent HostProvider could be imagined:
 
 Review comment:
   Fixed.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312868828
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
 ##
 @@ -212,341 +209,343 @@ public void processRequest(Request request) {
 throw ke;
 }
 
-LOG.debug("{}",request);
+LOG.debug("{}", request);
 
 if (request.isStale()) {
 ServerMetrics.getMetrics().STALE_REPLIES.add(1);
 }
 
 switch (request.type) {
-case OpCode.ping: {
-lastOp = "PING";
-updateStats(request, lastOp, lastZxid);
+case OpCode.ping: {
 
 Review comment:
   It is done by an auto formatting phase. For introduce such a phase many of 
'+'/'{' should be around by space could be fixed. It is a format related change 
and sure, no logical changes.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312868387
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
 ##
 @@ -188,24 +179,27 @@ public static EphemeralType get(long ephemeralOwner) {
 }
 
 /**
- * Make sure the given server ID is compatible with the current extended 
ephemeral setting
+ * Make sure the given server ID is compatible with the current extended 
ephemeral setting.
  *
  * @param serverId Server ID
  * @throws RuntimeException extendedTypesEnabled is true but Server ID is 
too large
  */
 public static void validateServerId(long serverId) {
-// TODO: in the future, serverId should be validated for all cases, 
not just the extendedEphemeralTypesEnabled case
+// TODO: in the future, serverId should be validated for all cases,
 
 Review comment:
   Support. This is also how akka community tracked TODOs and it was a good 
practice yet.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312868023
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
 ##
 @@ -89,8 +78,9 @@ public long toEphemeralOwner(long ttl) {
 if ((ttl > TTL.maxValue()) || (ttl <= 0)) {
 throw new IllegalArgumentException("ttl must be positive and 
cannot be larger than: " + TTL.maxValue());
 }
-//noinspection PointlessBitwiseExpression
 
 Review comment:
   Invalid any more.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312866880
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/client/HostProvider.java
 ##
 @@ -18,58 +18,60 @@
 
 package org.apache.zookeeper.client;
 
-import org.apache.yetus.audience.InterfaceAudience;
-
 import java.net.InetSocketAddress;
 import java.util.Collection;
+import org.apache.yetus.audience.InterfaceAudience;
 
 /**
  * A set of hosts a ZooKeeper client should connect to.
- * 
- * Classes implementing this interface must guarantee the following:
- * 
- * * Every call to next() returns an InetSocketAddress. So the iterator never
- * ends.
- * 
- * * The size() of a HostProvider may never be zero.
- * 
- * A HostProvider must return resolved InetSocketAddress instances on next() 
if the next address is resolvable.
+ *
+ * Classes implementing this interface must guarantee the following:
+ *
+ * 
+ * Every call to next() returns an InetSocketAddress. So the iterator 
never ends.
+ * The size() of a HostProvider may never be zero.
+ * 
+ *
+ * A HostProvider must return resolved InetSocketAddress instances on 
next() if the next address is resolvable.
  * In that case, it's up to the HostProvider, whether it returns the next 
resolvable address in the list or return
  * the next one as UnResolved.
- * 
- * Different HostProvider could be imagined:
- * 
- * * A HostProvider that loads the list of Hosts from an URL or from DNS 
- * * A HostProvider that re-resolves the InetSocketAddress after a timeout. 
- * * A HostProvider that prefers nearby hosts.
+ *
+ * ifferent HostProvider could be imagined:
 
 Review comment:
   Nice catch. Fixing...


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312865164
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java
 ##
 @@ -184,25 +191,31 @@ public static String validateFileInput(String filePath) {
 /**
  * Visits the subtree with root as given path and calls the passed 
callback with each znode
  * found during the search. It performs a depth-first, pre-order traversal 
of the tree.
- * 
- * Important: This is not an atomic snapshot of the tree 
ever, but the
+ *
+ * Important: This is not an atomic snapshot of the tree 
ever, but the
  * state as it exists across multiple RPCs from zkClient to the ensemble.
  * For practical purposes, it is suggested to bring the clients to the 
ensemble
  * down (i.e. prevent writes to pathRoot) to 'simulate' a snapshot 
behavior.
  */
-public static void visitSubTreeDFS(ZooKeeper zk, final String path, 
boolean watch,
-StringCallback cb) throws KeeperException, InterruptedException {
+public static void visitSubTreeDFS(
+ZooKeeper zk,
+final String path,
+boolean watch,
+StringCallback cb
+) throws KeeperException, InterruptedException {
 PathUtils.validatePath(path);
 
 zk.getData(path, watch, null);
 cb.processResult(Code.OK.intValue(), path, null, path);
 visitSubTreeDFSHelper(zk, path, watch, cb);
 }
 
-@SuppressWarnings("unchecked")
 
 Review comment:
   Yes. This is validated by the build cycle as we enable `-Werror`.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312864700
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -928,12 +925,12 @@ else if (serverPath.length() > chrootPath.length())
 packet.replyHeader.setErr(
 KeeperException.Code.CONNECTIONLOSS.intValue());
 throw new IOException("Xid out of order. Got Xid "
-+ replyHdr.getXid() + " with err " +
-+ replyHdr.getErr() +
-" expected Xid "
++ replyHdr.getXid() + " with err "
++ +replyHdr.getErr()
 
 Review comment:
   Here is a rule `+' should be on a new line`. For cleanup I guess you mean 
replace it with `String.format`? It would be a follow up to unify the 
log/Exception format and generation like that for javadoc(ZOOKEEPER-3469). This 
pull request would be a clean start point for all of this request but we should 
prevent it from too complex IMHO.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312863142
 
 

 ##
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
 ##
 @@ -160,7 +157,7 @@
 
 private long sessionId;
 
-private byte sessionPasswd[] = new byte[16];
+private byte[] sessionPasswd;
 
 Review comment:
   Right. Always be initialized in constructor.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [zookeeper] TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on a change in pull request #1049: ZOOKEEPER-3475 Enable 
Checkstyle configuration on zookeeper-server
URL: https://github.com/apache/zookeeper/pull/1049#discussion_r312809875
 
 

 ##
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMXBean.java
 ##
 @@ -25,192 +25,233 @@
 /**
  * @return the server socket port number
  */
-public String getClientPort();
+String getClientPort();
+
 /**
  * @return the zookeeper server version
  */
-public String getVersion();
+String getVersion();
+
 /**
  * @return time the server was started
  */
-public String getStartTime();
+String getStartTime();
+
 /**
  * @return min request latency in ms
  */
-public long getMinRequestLatency();
+long getMinRequestLatency();
+
 /**
  * @return average request latency in ms
  */
-public double getAvgRequestLatency();
+double getAvgRequestLatency();
+
 /**
  * @return max request latency in ms
  */
-public long getMaxRequestLatency();
+long getMaxRequestLatency();
+
 /**
  * @return number of packets received so far
  */
-public long getPacketsReceived();
+long getPacketsReceived();
+
 /**
  * @return number of packets sent so far
  */
-public long getPacketsSent();
+long getPacketsSent();
+
 /**
  * @return number of fsync threshold exceeds so far
  */
-public long getFsyncThresholdExceedCount();
+long getFsyncThresholdExceedCount();
+
 /**
  * @return number of outstanding requests.
  */
-public long getOutstandingRequests();
+long getOutstandingRequests();
+
 /**
- * Current TickTime of server in milliseconds
+ * Current TickTime of server in milliseconds.
  */
-public int getTickTime();
+int getTickTime();
+
 /**
- * Set TickTime of server in milliseconds
+ * Set TickTime of server in milliseconds.
  */
-public void setTickTime(int tickTime);
+void setTickTime(int tickTime);
 
-/** Current maxClientCnxns allowed from a particular host */
-public int getMaxClientCnxnsPerHost();
+/**
+ * Current maxClientCnxns allowed from a particular host.
+ */
+int getMaxClientCnxnsPerHost();
 
-/** Set maxClientCnxns allowed from a particular host */
-public void setMaxClientCnxnsPerHost(int max);
+/**
+ * Set maxClientCnxns allowed from a particular host.
+ */
+void setMaxClientCnxnsPerHost(int max);
 
 /**
- * Current minSessionTimeout of the server in milliseconds
+ * Current minSessionTimeout of the server in milliseconds.
  */
-public int getMinSessionTimeout();
+int getMinSessionTimeout();
+
 /**
- * Set minSessionTimeout of server in milliseconds
+ * Set minSessionTimeout of server in milliseconds.
  */
-public void setMinSessionTimeout(int min);
+void setMinSessionTimeout(int min);
 
 /**
- * Current maxSessionTimeout of the server in milliseconds
+ * Current maxSessionTimeout of the server in milliseconds.
  */
-public int getMaxSessionTimeout();
+int getMaxSessionTimeout();
+
 /**
- * Set maxSessionTimeout of server in milliseconds
+ * Set maxSessionTimeout of server in milliseconds.
  */
-public void setMaxSessionTimeout(int max);
+void setMaxSessionTimeout(int max);
+
+boolean getResponseCachingEnabled();
 
-public boolean getResponseCachingEnabled();
-public void setResponseCachingEnabled(boolean isEnabled);
+void setResponseCachingEnabled(boolean isEnabled);
 
 /* Connection throttling settings */
-public int getConnectionMaxTokens();
-public void setConnectionMaxTokens(int val);
 
 Review comment:
   We don't have a checkstyle rules on this case and so there is no so called 
standardized format in fact. It is done by an auto formatting phase. I can 
understand the pros grouping getters and setters. Fair enough to revert this. 
Thanks for your nice catch :P


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:
us...@infra.apache.org


With regards,
Apache Git Services