tahoe-lafs/trac

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

New issue

Closed

opened 2011-01-04 10:20:14 +00:00 by daira · 14 comments

daira commented

2011-01-04 10:20:14 +00:00

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 source: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").

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 source: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").

daira added the

labels 2011-01-04 10:20:14 +00:00

daira added this to the 1.9.0 milestone 2011-01-04 10:20:14 +00:00

daira commented

2011-01-04 11:23:12 +00:00

Author

Attachment rm-without-path.darcs.patch (12227 bytes) added

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

**Attachment** rm-without-path.darcs.patch (12227 bytes) added 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.patch

12 KiB

daira modified the milestone from 1.9.0 to 1.8.2

2011-01-06 00:32:53 +00:00

warner commented

2011-01-17 10:01:41 +00:00

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

daira self-assigned this 2011-01-18 00:56:41 +00:00

warner commented

2011-01-21 17:07:25 +00:00

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.

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.

warner modified the milestone from 1.8.2 to 1.9.0

2011-01-21 17:07:25 +00:00

zooko commented

2011-01-21 17:49:28 +00:00

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

daira was unassigned by zooko

2011-01-21 17:50:01 +00:00

warner was assigned by zooko

2011-01-21 17:50:01 +00:00

warner commented

2011-01-21 22:08:19 +00:00

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

warner commented

2011-01-21 22:16:33 +00:00

assigning back to davidsarah to write docs/deleting.rst

assigning back to davidsarah to write `docs/deleting.rst`

warner removed their assignment 2011-01-21 22:16:33 +00:00

daira was assigned by warner

2011-01-21 22:16:33 +00:00

daira commented

2011-01-22 07:05:25 +00:00

Author

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.

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.

daira commented

2011-01-22 07:15:18 +00:00

Author

Attachment rm-without-path.darcs.2.patch (28702 bytes) added

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

**Attachment** rm-without-path.darcs.2.patch (28702 bytes) added test_cli.py: Test for error message when 'tahoe rm' is invoked without a path. refs #1292

rm-without-path.darcs.2.patch

28 KiB

daira commented

2011-01-23 03:16:46 +00:00

Author

(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.)

(Assigning to warner to decide whether to apply [attachment:rm-without-path.darcs.2.patch](/tahoe-lafs/trac/attachments/000078ac-8dad-097d-5f7a-027d611850fa) . I will still write `docs/deleting.rst` if I have time.)

daira removed their assignment 2011-01-23 03:16:46 +00:00

warner was assigned by daira

2011-01-23 03:16:46 +00:00

zooko commented

2011-06-23 21:32:24 +00:00

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!

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!

warner commented

2011-08-01 02:06:31 +00:00

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.