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

          Harsh J created issue -
          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.
          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.
          Harsh J made changes -
          Field Original Value New Value
          Attachment ZOOKEEPER-1326.patch [ 12508661 ]
          Harsh J made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          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
          Harsh J made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          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.
          Harsh J made changes -
          Attachment ZOOKEEPER-1326.patch [ 12508663 ]
          Harsh J 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/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.
          Patrick Hunt made changes -
          Assignee Harsh J [ qwertymaniac ]
          Patrick Hunt made changes -
          Fix Version/s 3.5.0 [ 12316644 ]
          Hide
          Patrick Hunt added a comment -

          Looks good, thanks Harsh!

          Show
          Patrick Hunt added a comment - Looks good, thanks Harsh!
          Patrick Hunt made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Resolution Fixed [ 1 ]
          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
          Harsh J made changes -
          Summary The CLI commands "delete" and "rmr" are confusing. Can we have "rm" + "rmr" instead? The CLI commands "delete" and "rmr" are confusing. Can we have "delete" + "deleteall" instead?
          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
          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
          Harsh J made changes -
          Link This issue supercedes ZOOKEEPER-729 [ ZOOKEEPER-729 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Patch Available Patch Available Open Open
          8m 28s 1 Harsh J 27/Dec/11 05:39
          Open Open Patch Available Patch Available
          15d 9h 53m 2 Harsh J 27/Dec/11 05:57
          Patch Available Patch Available Resolved Resolved
          17h 56m 1 Patrick Hunt 27/Dec/11 23:53

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development