ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-271

Better command line parsing in ZookeeperMain.

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.0, 3.0.1
    • Fix Version/s: 3.5.0
    • Component/s: java client
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Hide
      There are three incompatibilities introduced by this commit into the client shell:

      1) now requires commons-cli
      2) get no longer returns stat information by default, however there is a "-s" option that will result in the stat being included
      3) a deprecated message is reported in some cases, when the old command format is used. As a result the output of the command may be different compared to client output prior to this change.
      Show
      There are three incompatibilities introduced by this commit into the client shell: 1) now requires commons-cli 2) get no longer returns stat information by default, however there is a "-s" option that will result in the stat being included 3) a deprecated message is reported in some cases, when the old command format is used. As a result the output of the command may be different compared to client output prior to this change.

      Description

      The command line parsing in zookeepermain is very basic.We should use some kind of cli parsing (commons-cli?) or something else that is standard and improve our command line parsing. This will remove the scattered code that we have in zookeepermain and we will have much better command line parsing.

      1. ZOOKEEPER-271.patch
        77 kB
        Hartmut Lang
      2. ZOOKEEPER-271-1.patch
        77 kB
        Hartmut Lang

        Issue Links

          Activity

          Hide
          Patrick Hunt added a comment -

          not a blocker for 3.2, moving to 3.3

          Show
          Patrick Hunt added a comment - not a blocker for 3.2, moving to 3.3
          Hide
          Hartmut Lang added a comment -

          A attached a patch with my work to refactor the CLI to use commons-cli.
          Every command has now its own java-class, with CliCommand.java as base.class.

          The options for some commands were changed to match the option usage in commons-cli.
          But to maintain the compatibility the old argument style for these commands was preserved. This could be removed in a future revision.

          The commands with changed options are:
          get [-s] [-w] path, old version was: get path [watch]
          ls [-w] path, old version was ls path [watch]
          ls2 [-w] path, old version was ls2 path [watch]
          stat [-w] path, old version was stat path [watch]

          Please have a look.

          Show
          Hartmut Lang added a comment - A attached a patch with my work to refactor the CLI to use commons-cli. Every command has now its own java-class, with CliCommand.java as base.class. The options for some commands were changed to match the option usage in commons-cli. But to maintain the compatibility the old argument style for these commands was preserved. This could be removed in a future revision. The commands with changed options are: get [-s] [-w] path, old version was: get path [watch] ls [-w] path, old version was ls path [watch] ls2 [-w] path, old version was ls2 path [watch] stat [-w] path, old version was stat path [watch] Please have a look.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12517001/ZOOKEEPER-271-1.patch
          against trunk revision 1296035.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 findbugs. The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/973//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/973//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/973//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12517001/ZOOKEEPER-271-1.patch against trunk revision 1296035. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 3 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/973//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/973//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/973//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12517198/ZOOKEEPER-271.patch
          against trunk revision 1296035.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/978//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/978//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/978//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12517198/ZOOKEEPER-271.patch against trunk revision 1296035. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/978//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/978//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/978//console This message is automatically generated.
          Hide
          Hartmut Lang added a comment -

          Please review and commit.

          Show
          Hartmut Lang added a comment - Please review and commit.
          Hide
          Patrick Hunt added a comment -

          Nice job! I'm reviewing now. Question though:

          The options for some commands were changed to match the option usage in commons-cli.
          But to maintain the compatibility the old argument style for these commands was preserved. This could be removed in a future revision.
          

          does this mean the new client is 100% backward compatible with the old client or not? If not we should mark this as an incompatible change (there's a flag for this) and document in the "release notes".

          Show
          Patrick Hunt added a comment - Nice job! I'm reviewing now. Question though: The options for some commands were changed to match the option usage in commons-cli. But to maintain the compatibility the old argument style for these commands was preserved. This could be removed in a future revision. does this mean the new client is 100% backward compatible with the old client or not? If not we should mark this as an incompatible change (there's a flag for this) and document in the "release notes".
          Hide
          Hartmut Lang added a comment -

          Thanks for reviewing it.
          I added backward compatibility to all the commands.
          So it should be 100% backward compatible, but a warning is printed.

          Show
          Hartmut Lang added a comment - Thanks for reviewing it. I added backward compatibility to all the commands. So it should be 100% backward compatible, but a warning is printed.
          Hide
          Patrick Hunt added a comment -

          Sweet. Thanks Hartmut. I've committed this to trunk (3.5.0) and noted a few issues I found in the release notes.

          Show
          Patrick Hunt added a comment - Sweet. Thanks Hartmut. I've committed this to trunk (3.5.0) and noted a few issues I found in the release notes.
          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #1498 (See https://builds.apache.org/job/ZooKeeper-trunk/1498/)
          ZOOKEEPER-271. Better command line parsing in ZookeeperMain. (Hartmut Lang via phunt) (Revision 1302068)

          Result = SUCCESS
          phunt : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1302068
          Files :

          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/ivy.xml
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeperMain.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/AclParser.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/AddAuthCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/CliCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/CloseCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/CreateCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/DelQuotaCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/DeleteAllCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/DeleteCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/GetAclCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/GetCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/ListQuotaCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/Ls2Command.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/LsCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/SetAclCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/SetCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/SetQuotaCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/StatCommand.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/StatPrinter.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/SyncCommand.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/ZooKeeperTest.java
          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #1498 (See https://builds.apache.org/job/ZooKeeper-trunk/1498/ ) ZOOKEEPER-271 . Better command line parsing in ZookeeperMain. (Hartmut Lang via phunt) (Revision 1302068) Result = SUCCESS phunt : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1302068 Files : /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/ivy.xml /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeperMain.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/AclParser.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/AddAuthCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/CliCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/CloseCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/CreateCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/DelQuotaCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/DeleteAllCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/DeleteCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/GetAclCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/GetCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/ListQuotaCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/Ls2Command.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/LsCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/SetAclCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/SetCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/SetQuotaCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/StatCommand.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/StatPrinter.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/cli/SyncCommand.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/ZooKeeperTest.java

            People

            • Assignee:
              Hartmut Lang
              Reporter:
              Mahadev konar
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development