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-1.patch
        77 kB
        Hartmut Lang
      2. ZOOKEEPER-271.patch
        77 kB
        Hartmut Lang

        Issue Links

          Activity

          Mahadev konar created issue -
          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
          Patrick Hunt made changes -
          Field Original Value New Value
          Fix Version/s 3.3.0 [ 12313976 ]
          Fix Version/s 3.2.0 [ 12313491 ]
          Patrick Hunt made changes -
          Fix Version/s 3.4.0 [ 12314469 ]
          Fix Version/s 3.3.0 [ 12313976 ]
          Mahadev konar made changes -
          Fix Version/s 3.5.0 [ 12316644 ]
          Fix Version/s 3.4.0 [ 12314469 ]
          Hartmut Lang made changes -
          Attachment ZOOKEEPER-271-1.patch [ 12517001 ]
          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.
          Camille Fournier made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          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.
          Patrick Hunt made changes -
          Assignee Hartmut Lang [ hlang ]
          Hartmut Lang made changes -
          Status Patch Available [ 10002 ] In Progress [ 3 ]
          Hartmut Lang made changes -
          Attachment ZOOKEEPER-271.patch [ 12517198 ]
          Hartmut Lang made changes -
          Status In Progress [ 3 ] Patch Available [ 10002 ]
          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.
          Hartmut Lang made changes -
          Link This issue blocks ZOOKEEPER-1408 [ ZOOKEEPER-1408 ]
          Hartmut Lang made changes -
          Link This issue blocks ZOOKEEPER-1409 [ ZOOKEEPER-1409 ]
          Hartmut Lang made changes -
          Link This issue supercedes ZOOKEEPER-1307 [ ZOOKEEPER-1307 ]
          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.
          Patrick Hunt made changes -
          Hadoop Flags Incompatible change,Reviewed [ 10342,10343 ]
          Release Note 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.
          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.
          Patrick Hunt made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          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
          Gavin made changes -
          Link This issue blocks ZOOKEEPER-1408 [ ZOOKEEPER-1408 ]
          Gavin made changes -
          Link This issue is depended upon by ZOOKEEPER-1408 [ ZOOKEEPER-1408 ]
          Gavin made changes -
          Link This issue blocks ZOOKEEPER-1409 [ ZOOKEEPER-1409 ]
          Gavin made changes -
          Link This issue is depended upon by ZOOKEEPER-1409 [ ZOOKEEPER-1409 ]

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development