----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49112/#review139157 -----------------------------------------------------------
Looks pretty reasonable to me, however I do have a few high level questions on how far we might/should go. Thoughts? Note that the patch doesn't apply to 3.4. Not sure if it's worth fixing there? src/java/main/org/apache/zookeeper/ZooKeeperMain.java (line 626) <https://reviews.apache.org/r/49112/#comment204310> See my comment on the exception class. Should we have more than one exit code, ie based on the type of exception. I'm not sure if that's valuable, but it seems like it might be for someone trying to script this? src/java/main/org/apache/zookeeper/cli/InvalidCommandException.java (line 20) <https://reviews.apache.org/r/49112/#comment204308> This seems very specific. Perhaps we can have a general base cli exception and this would be a subclass of that? The base exception could also provide the exit code, rather than hardcoding it as we have now.... Not sure if this would work but we could also have a subclass that understands keeper exceptions? - Patrick Hunt On June 22, 2016, 10:27 p.m., Abraham Fine wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49112/ > ----------------------------------------------------------- > > (Updated June 22, 2016, 10:27 p.m.) > > > Review request for zookeeper. > > > Bugs: ZOOKEEPER-1898 > https://issues.apache.org/jira/browse/ZOOKEEPER-1898 > > > Repository: zookeeper-git > > > Description > ------- > > zookeeper-cli always return "0" as exit code > > > Diffs > ----- > > src/java/main/org/apache/zookeeper/ZooKeeperMain.java f722693 > src/java/main/org/apache/zookeeper/cli/CliCommand.java 527a19e > src/java/main/org/apache/zookeeper/cli/CreateCommand.java cc96939 > src/java/main/org/apache/zookeeper/cli/DeleteCommand.java 4714be5 > src/java/main/org/apache/zookeeper/cli/InvalidCommandException.java > PRE-CREATION > src/java/main/org/apache/zookeeper/cli/ListQuotaCommand.java 907c466 > src/java/main/org/apache/zookeeper/cli/LsCommand.java 43099b8 > src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java 282af55 > src/java/main/org/apache/zookeeper/cli/RemoveWatchesCommand.java f63cf0f > src/java/main/org/apache/zookeeper/cli/SetAclCommand.java f27446c > src/java/main/org/apache/zookeeper/cli/SetCommand.java 1c39377 > src/java/main/org/apache/zookeeper/cli/SetQuotaCommand.java 873c6d2 > src/java/test/org/apache/zookeeper/ZooKeeperTest.java 574eab0 > > Diff: https://reviews.apache.org/r/49112/diff/ > > > Testing > ------- > > > Thanks, > > Abraham Fine > >
