Uploaded image for project: 'ZooKeeper'
  1. ZooKeeper
  2. ZOOKEEPER-271

Better command line parsing in ZookeeperMain.

Details

    • Improvement
    • Status: Resolved
    • Minor
    • Resolution: Fixed
    • 3.0.0, 3.0.1
    • 3.5.0
    • java client
    • None
    • Incompatible change, Reviewed
    • 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.

      Attachments

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

        Issue Links

          Activity

            not a blocker for 3.2, moving to 3.3

            phunt Patrick D. Hunt added a comment - not a blocker for 3.2, moving to 3.3
            hlang 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.

            hlang 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.
            hadoopqa 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.

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

            hadoopqa 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.
            hlang Hartmut Lang added a comment -

            Please review and commit.

            hlang Hartmut Lang added a comment - Please review and commit.

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

            phunt Patrick D. 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".
            hlang 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.

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

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

            phunt Patrick D. 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.
            hudson 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
            hudson 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

              hlang Hartmut Lang
              mahadev Mahadev Konar
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: