#1292 closed defect (fixed)

'tahoe rm' without a path gives an AssertionError with no explanation

Reported by: davidsarah Owned by: warner
Priority: major Milestone: 1.9.0
Component: code-frontend-cli Version: 1.8.1
Keywords: error usability tahoe-rm docs easy docs-needed reviewed Cc:
Launchpad Bug:

Description

Reported by drewp on IRC:

A new user tried to do a tahoe rm on a mutable file (SSK URI) without a path. The error he got was an assertion failure at src/allmydata/scripts/tahoe_rm.py#L24.

tahoe rm only works on directory entries, and therefore requires a path after the URI or alias, but that is non-obvious.

For a mutable file, it would be possible to truncate the file or write a marker in place of it, although that would not necessarily delete all shares. This ticket is just for the error message, and maybe documentation about what a user can do in this situation.

See also #776 (users are confused by "tahoe rm").

Attachments (2)

rm-without-path.darcs.patch (11.9 KB) - added by davidsarah at 2011-01-04T11:23:12Z.
tahoe_rm.py: better error message when there is no path. docs/deleting.rst needs to be written. Includes test. refs #1292
rm-without-path.darcs.2.patch (28.0 KB) - added by davidsarah at 2011-01-22T07:15:18Z.
test_cli.py: Test for error message when 'tahoe rm' is invoked without a path. refs #1292

Download all attachments as: .zip

Change History (20)

Changed at 2011-01-04T11:23:12Z by davidsarah

tahoe_rm.py: better error message when there is no path. docs/deleting.rst needs to be written. Includes test. refs #1292

comment:1 Changed at 2011-01-04T11:25:05Z by davidsarah

  • Keywords review-needed added

comment:2 Changed at 2011-01-06T00:32:53Z by davidsarah

  • Milestone changed from 1.9.0 to 1.8.2

comment:3 Changed at 2011-01-17T10:01:41Z by warner

code change and test looks great. But if it references docs/deleting.rst, then it needs to be written.

comment:4 Changed at 2011-01-18T00:56:41Z by davidsarah

  • Keywords review-needed removed
  • Owner set to davidsarah
  • Status changed from new to assigned

comment:5 Changed at 2011-01-18T08:19:42Z by zooko

  • Cc r added
  • Keywords docs-needed reviewed added

comment:6 Changed at 2011-01-19T04:38:15Z by zooko

  • Cc r removed

comment:7 Changed at 2011-01-21T17:07:25Z by warner

  • Milestone changed from 1.8.2 to 1.9.0

I'm not seeing any progress towards a docs/deleting.rst, so I'm kicking this out of 1.8.2 . We can land it as soon as we've got those docs.

comment:8 Changed at 2011-01-21T17:49:28Z by zooko

Hey, I thought we could fix docs after the code freeze.

comment:9 Changed at 2011-01-21T17:50:01Z by zooko

  • Owner changed from davidsarah to warner
  • Status changed from assigned to new

comment:10 Changed at 2011-01-21T22:08:19Z by warner

Hmmm. I'm happy to have docs get fixed or added after the code freeze, but I'm not excited about landing a code change which explicitly references a missing piece of documentation. I wouldn't want to ship this change without the docs, and given the non-trivial challenge of explaining what "delete" means in an object-graph FS to a traditional-FS audience, I'm not confident that the documentation will appear before the release. And then I'd be faced with deciding between yanking the code change on the day of the release (way after code freeze) vs shipping with an embarassing dangling docs-pointer.

Once we get the docs, the release manager may choose to accept the patch during freeze (it is pretty small, after all).

comment:11 Changed at 2011-01-21T22:16:33Z by warner

  • Owner changed from warner to davidsarah

assigning back to davidsarah to write docs/deleting.rst

comment:12 Changed at 2011-01-22T07:05:25Z by davidsarah

How about we land something that omits the reference to deleting.rst, but fixes the assert -- for example using just the string "'tahoe rm' can only unlink directory entries, so a path must be given." Then we can change that string if the docs are written before the final release.

Changed at 2011-01-22T07:15:18Z by davidsarah

test_cli.py: Test for error message when 'tahoe rm' is invoked without a path. refs #1292

comment:13 Changed at 2011-01-23T03:16:46Z by davidsarah

  • Owner changed from davidsarah to warner

(Assigning to warner to decide whether to apply attachment:rm-without-path.darcs.2.patch . I will still write docs/deleting.rst if I have time.)

Last edited at 2011-01-23T03:18:42Z by davidsarah (previous) (diff)

comment:14 Changed at 2011-06-23T21:32:24Z by zooko

  • Keywords review-needed added; reviewed removed

Removing reviewed since I don't need to push this patch to trunk and adding review-needed since someone (Brian) needs to answer a question about what to do. There, now there are no tickets left with reviewed on them. Send more reviewed tickets!

Last edited at 2011-06-23T21:40:18Z by zooko (previous) (diff)

comment:15 Changed at 2011-08-01T02:06:31Z by warner

  • Keywords reviewed added; review-needed removed

patch2 looks good, sorry about the delay. The error message is clear enough for me. Explaining how deletion works in general can live in another ticket and be written another day, now that this patch doesn't imply it's preexistence.

comment:16 Changed at 2011-08-01T02:13:48Z by david-sarah@…

In 00fefeba4969605d:

test_cli.py: Test for error message when 'tahoe rm' is invoked without a path. refs #1292

comment:17 Changed at 2011-08-01T02:13:49Z by david-sarah@…

In 52963f4a76809cde:

tahoe_rm.py: better error message when there is no path. refs #1292

comment:18 Changed at 2011-08-01T02:17:17Z by warner

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.