Skip to content

Commit

Permalink
ceph.in: print all matched commands if arg missing
Browse files Browse the repository at this point in the history
so if only a single command matches, the "Invalid command:" error
message is
printed, otherwise, the top 10 matched candidate commands are printed.

also, validate_command() now always returns `{}` if the command is not
valid. because we just the retval of it using
```py
if validate_command(...):
  # ...
```
and we do not differentiate `None` from `{}` here. and it'd be more
consistent and easier from the implementation perspective this way.

Fixes: http://tracker.ceph.com/issues/22344
Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit ab076ed)
  • Loading branch information
tchaikov committed Mar 1, 2018
1 parent d7ff3ce commit ac29480
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 15 deletions.
30 changes: 22 additions & 8 deletions src/pybind/ceph_argparse.py
Expand Up @@ -53,6 +53,13 @@ class ArgumentFormat(ArgumentError):
pass


class ArgumentMissing(ArgumentError):
"""
Argument value missing in a command
"""
pass


class ArgumentValid(ArgumentError):
"""
Argument value is otherwise invalid (doesn't match choices, for instance)
Expand Down Expand Up @@ -944,7 +951,7 @@ def validate(args, signature, flags=0, partial=False):
return d
# special-case the "0 expected 1" case
if desc.numseen == 0 and desc.n == 1:
raise ArgumentNumber(
raise ArgumentMissing(
'missing required parameter {0}'.format(desc)
)
raise ArgumentNumber(
Expand Down Expand Up @@ -1042,6 +1049,7 @@ def validate_command(sigdict, args, verbose=False):
print("bestcmds_sorted: ", file=sys.stderr)
pprint.PrettyPrinter(stream=sys.stderr).pprint(bestcmds_sorted)

e = None
# for everything in bestcmds, look for a true match
for cmdsig in bestcmds_sorted:
for cmd in cmdsig.values():
Expand All @@ -1054,6 +1062,10 @@ def validate_command(sigdict, args, verbose=False):
# ignore prefix mismatches; we just haven't found
# the right command yet
pass
except ArgumentMissing as e:
if len(bestcmds) == 1:
found = cmd
break
except ArgumentTooFew:
# It looked like this matched the beginning, but it
# didn't have enough args supplied. If we're out of
Expand All @@ -1066,23 +1078,25 @@ def validate_command(sigdict, args, verbose=False):
# Solid mismatch on an arg (type, range, etc.)
# Stop now, because we have the right command but
# some other input is invalid
print("Invalid command: ", e, file=sys.stderr)
print(concise_sig(sig), ': ', cmd['help'], file=sys.stderr)
return {}
if found:
found = cmd
break
if found or e:
break

if not found:
if found:
if not valid_dict:
print("Invalid command:", e, file=sys.stderr)
print(concise_sig(sig), ': ', cmd['help'], file=sys.stderr)
else:
bestcmds = bestcmds[:10]
print('no valid command found; {0} closest matches:'.format(len(bestcmds)), file=sys.stderr)
for cmdsig in bestcmds:
for (cmdtag, cmd) in cmdsig.items():
print(concise_sig(cmd['sig']), file=sys.stderr)
return None

return valid_dict



def find_cmd_target(childargs):
"""
Using a minimal validation, figure out whether the command
Expand Down
40 changes: 33 additions & 7 deletions src/test/pybind/test_ceph_argparse.py
Expand Up @@ -22,7 +22,9 @@

import os
import re
import sys
import json
from StringIO import StringIO

def get_command_descriptions(what):
CEPH_BIN = os.environ['CEPH_BIN']
Expand All @@ -45,8 +47,7 @@ class TestArgparse:

def assert_valid_command(self, args):
result = validate_command(sigdict, args)
assert_not_equal(result,None)
assert_not_equal(result,{})
assert_not_in(result, [{}, None])

def check_1_natural_arg(self, prefix, command):
self.assert_valid_command([prefix, command, '1'])
Expand Down Expand Up @@ -88,20 +89,34 @@ def check_no_arg(self, prefix, command):
command,
'toomany']))

def capture_output(self, args, stdout=None, stderr=None):
if stdout:
stdout = StringIO()
sys.stdout = stdout
if stderr:
stderr = StringIO()
sys.stderr = stderr
ret = validate_command(sigdict, args)
if stdout:
stdout = stdout.getvalue().strip()
if stderr:
stderr = stderr.getvalue().strip()
return ret, stdout, stderr


class TestBasic:

def test_non_ascii_in_non_options(self):
# ArgumentPrefix("no match for {0}".format(s)) is not able to convert
# unicode str parameter into str. and validate_command() should not
# choke on it.
assert_is_none(validate_command(sigdict, [u'章鱼和鱿鱼']))
assert_is_none(validate_command(sigdict, [u'–w']))
assert_equal({}, validate_command(sigdict, [u'章鱼和鱿鱼']))
assert_equal({}, validate_command(sigdict, [u'–w']))
# actually we always pass unicode strings to validate_command() in "ceph"
# CLI, but we also use bytestrings in our tests, so make sure it does not
# break.
assert_is_none(validate_command(sigdict, ['章鱼和鱿鱼']))
assert_is_none(validate_command(sigdict, ['–w']))
assert_equal({}, validate_command(sigdict, ['章鱼和鱿鱼']))
assert_equal({}, validate_command(sigdict, ['–w']))


class TestPG(TestArgparse):
Expand Down Expand Up @@ -180,6 +195,17 @@ def test_debug(self):
assert_equal({}, validate_command(sigdict, ['pg', 'debug',
'invalid']))

def test_pg_missing_args_output(self):
ret, _, stderr = self.capture_output(['pg'], stderr=True)
assert_equal({}, ret)
assert_regexp_matches(stderr, re.compile('no valid command found.* closest matches'))

def test_pg_wrong_arg_output(self):
ret, _, stderr = self.capture_output(['pg', 'map', 'bad-pgid'],
stderr=True)
assert_equal({}, ret)
assert_in("Invalid command", stderr)


class TestAuth(TestArgparse):

Expand Down Expand Up @@ -872,7 +898,7 @@ def test_set_unset(self):
'pause', 'toomany']))

def test_cluster_snap(self):
assert_equal(None, validate_command(sigdict, ['osd', 'cluster_snap']))
assert_equal({}, validate_command(sigdict, ['osd', 'cluster_snap']))

def test_down(self):
self.check_1_or_more_string_args('osd', 'down')
Expand Down

0 comments on commit ac29480

Please sign in to comment.