ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1326

The CLI commands "delete" and "rmr" are confusing. Can we have "delete" + "deleteall" instead?

    Details

    • Type: Wish Wish
    • Status: Resolved
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 3.4.0
    • Fix Version/s: 3.5.0
    • Component/s: java client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      ZOOKEEPER-729 introduced 'rmr' for recursive 'delete' operations on a given node. Going by the unix convention, wouldn't it be much better if we were to have an 'rm' if there was an 'rmr' added?

      The current set is confusing. Or should we have 'delete' and 'deleteall' or summat?

      I know this is a nitpick, but I just dislike to see bad keywords used for commands.

      I'm OK to produce a backwards-compatible patch if this is acceptable.

      1. ZOOKEEPER-1326.patch
        3 kB
        Harsh J
      2. ZOOKEEPER-1326.patch
        3 kB
        Harsh J

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #1410 (See https://builds.apache.org/job/ZooKeeper-trunk/1410/)
          ZOOKEEPER-1326. The CLI commands "delete" and "rmr" are confusing. Can we have "rm" + "rmr" instead? Regenned docs. (Harsh J via phunt)
          ZOOKEEPER-1326. The CLI commands "delete" and "rmr" are confusing. Can we have "rm" + "rmr" instead? (Harsh J via phunt)

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

          • /zookeeper/trunk/docs/bookkeeperConfig.pdf
          • /zookeeper/trunk/docs/bookkeeperOverview.pdf
          • /zookeeper/trunk/docs/bookkeeperProgrammer.pdf
          • /zookeeper/trunk/docs/bookkeeperStarted.pdf
          • /zookeeper/trunk/docs/bookkeeperStream.pdf
          • /zookeeper/trunk/docs/index.pdf
          • /zookeeper/trunk/docs/javaExample.pdf
          • /zookeeper/trunk/docs/linkmap.pdf
          • /zookeeper/trunk/docs/recipes.pdf
          • /zookeeper/trunk/docs/releasenotes.pdf
          • /zookeeper/trunk/docs/zookeeperAdmin.pdf
          • /zookeeper/trunk/docs/zookeeperHierarchicalQuorums.pdf
          • /zookeeper/trunk/docs/zookeeperInternals.pdf
          • /zookeeper/trunk/docs/zookeeperJMX.pdf
          • /zookeeper/trunk/docs/zookeeperObservers.pdf
          • /zookeeper/trunk/docs/zookeeperOver.pdf
          • /zookeeper/trunk/docs/zookeeperProgrammers.pdf
          • /zookeeper/trunk/docs/zookeeperQuotas.pdf
          • /zookeeper/trunk/docs/zookeeperStarted.html
          • /zookeeper/trunk/docs/zookeeperStarted.pdf
          • /zookeeper/trunk/docs/zookeeperTutorial.pdf

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

          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/src/docs/src/documentation/content/xdocs/zookeeperStarted.xml
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeperMain.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/ZooKeeperTest.java
          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #1410 (See https://builds.apache.org/job/ZooKeeper-trunk/1410/ ) ZOOKEEPER-1326 . The CLI commands "delete" and "rmr" are confusing. Can we have "rm" + "rmr" instead? Regenned docs. (Harsh J via phunt) ZOOKEEPER-1326 . The CLI commands "delete" and "rmr" are confusing. Can we have "rm" + "rmr" instead? (Harsh J via phunt) phunt : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1225053 Files : /zookeeper/trunk/docs/bookkeeperConfig.pdf /zookeeper/trunk/docs/bookkeeperOverview.pdf /zookeeper/trunk/docs/bookkeeperProgrammer.pdf /zookeeper/trunk/docs/bookkeeperStarted.pdf /zookeeper/trunk/docs/bookkeeperStream.pdf /zookeeper/trunk/docs/index.pdf /zookeeper/trunk/docs/javaExample.pdf /zookeeper/trunk/docs/linkmap.pdf /zookeeper/trunk/docs/recipes.pdf /zookeeper/trunk/docs/releasenotes.pdf /zookeeper/trunk/docs/zookeeperAdmin.pdf /zookeeper/trunk/docs/zookeeperHierarchicalQuorums.pdf /zookeeper/trunk/docs/zookeeperInternals.pdf /zookeeper/trunk/docs/zookeeperJMX.pdf /zookeeper/trunk/docs/zookeeperObservers.pdf /zookeeper/trunk/docs/zookeeperOver.pdf /zookeeper/trunk/docs/zookeeperProgrammers.pdf /zookeeper/trunk/docs/zookeeperQuotas.pdf /zookeeper/trunk/docs/zookeeperStarted.html /zookeeper/trunk/docs/zookeeperStarted.pdf /zookeeper/trunk/docs/zookeeperTutorial.pdf phunt : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1225052 Files : /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/docs/src/documentation/content/xdocs/zookeeperStarted.xml /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeperMain.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/ZooKeeperTest.java
          Hide
          Patrick Hunt added a comment -

          Many more to follow I hope. thx.

          Show
          Patrick Hunt added a comment - Many more to follow I hope. thx.
          Hide
          Harsh J added a comment -

          Thanks Patrick and Mahadev! I believe this was my first ZK patch ever, whee

          Show
          Harsh J added a comment - Thanks Patrick and Mahadev! I believe this was my first ZK patch ever, whee
          Hide
          Patrick Hunt added a comment -

          Looks good, thanks Harsh!

          Show
          Patrick Hunt added a comment - Looks good, thanks Harsh!
          Hide
          Hadoop QA added a comment -

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

          +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/858//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/858//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/858//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/12508663/ZOOKEEPER-1326.patch against trunk revision 1222816. +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/858//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/858//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/858//console This message is automatically generated.
          Hide
          Harsh J added a comment -

          Patch that changes 'rmr' to 'deleteall' instead.

          This is more consistent with the API call ZooKeeper#delete.

          Also changes the tests to parse commands, like testDelete does, and adds a documentation line for this new command that was not previously added.

          Show
          Harsh J added a comment - Patch that changes 'rmr' to 'deleteall' instead. This is more consistent with the API call ZooKeeper#delete . Also changes the tests to parse commands, like testDelete does, and adds a documentation line for this new command that was not previously added.
          Hide
          Harsh J added a comment -

          Not sure what I was thinking when I wrote that. The consistency is achieved the other way round

          Show
          Harsh J added a comment - Not sure what I was thinking when I wrote that. The consistency is achieved the other way round
          Hide
          Harsh J added a comment -

          Attached patch switches over from 'delete' to 'rm' in a deprecating and non-breaking fashion.

          Changes existing tests to use 'rm' as well.

          Show
          Harsh J added a comment - Attached patch switches over from 'delete' to 'rm' in a deprecating and non-breaking fashion. Changes existing tests to use 'rm' as well.
          Hide
          Mahadev konar added a comment -

          @Harsh,
          Good point. A backwards compatible patch is welcome.

          Show
          Mahadev konar added a comment - @Harsh, Good point. A backwards compatible patch is welcome.

            People

            • Assignee:
              Harsh J
              Reporter:
              Harsh J
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development