Bug #6384
ceph tool: incorrect pool delete line returns unintelligible failure message
0%
Description
gregf@kai:~/ceph/src [wip-6189-cache-promote]$ ./ceph osd pool delete base_pool *** DEVELOPER MODE: setting PATH, PYTHONPATH and LD_LIBRARY_PATH *** Invalid command: saw 0 of pool2(<poolname>), expected 1 Error EINVAL: invalid command
Going through ./ceph --help I managed to figure out the proper format is
gregf@kai:~/ceph/src [wip-6189-cache-promote]$ ./ceph osd pool delete base_pool base_pool --yes-i-really-really-mean-it *** DEVELOPER MODE: setting PATH, PYTHONPATH and LD_LIBRARY_PATH *** pool 'base_pool' deleted
but the error output certainly didn't help me get there.
Associated revisions
ceph_argparse.py: clean up error reporting when required param missing
Treat "need 1, got 0" as a special case, and change the message to
"missing required parameter <x>". Also, when failing for that reason,
print the command concise description and its helptext.
Fixes: #6384
Signed-off-by: Dan Mick <dan.mick@inktank.com>
History
#1 Updated by Ian Colle over 10 years ago
- Assignee set to Dan Mick
- Priority changed from Normal to High
#2 Updated by Greg Farnum over 10 years ago
This is at least helped by commit:7e1c73538f44ad83db476ffa15d5121470b5154e in wip-6332 (pull request: https://github.com/ceph/ceph/pull/606).
#3 Updated by Dan Mick over 10 years ago
Not sure what you'd want to happen here; the "semantic reason for the failure of the command" is a bit of a stretch. We could maybe print the help for the failing command if there's only one that could have matched?....
#4 Updated by Greg Farnum over 10 years ago
Yeah, that would probably be good. It's clearly gotten to the point that it knows what command it's looking at, but while as a developer I eventually figured out what was going on, "saw 0 of pool2(<poolname>), expected 1" is meaningless without the source.
#5 Updated by Dan Mick over 10 years ago
Yeah, it's not the best message. Maybe we should special case the 'saw 0 expected 1' case to something more intelligible, like "must supply <pool2>" (since it's by far the
most common case)
#6 Updated by Dan Mick over 10 years ago
With both those suggestions implemented, the output looks like this:
$ ./ceph osd pool delete data *** DEVELOPER MODE: setting PATH, PYTHONPATH and LD_LIBRARY_PATH *** Invalid command: missing required parameter pool2(<poolname>) osd pool delete <poolname> <poolname> --yes-i-really-really-mean-it: delete pool (say pool twice, add --yes-i-really-really-mean-it) Error EINVAL: invalid command
Does that seem like a reasonable result?
#7 Updated by Dan Mick over 10 years ago
well, I've created a pull request; if you want to review, now's your chance
#8 Updated by Dan Mick over 10 years ago
- Status changed from New to Fix Under Review
#9 Updated by Dan Mick over 10 years ago
- Status changed from Fix Under Review to Resolved