Merge ~adam-collard/maas:maas-cli-tweaks into maas:master

Proposed by Adam Collard
Status: Merged
Approved by: Adam Collard
Approved revision: d2d41a970bd71f60375bad34a00f701b07a6beac
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~adam-collard/maas:maas-cli-tweaks
Merge into: maas:master
Diff against target: 165 lines (+60/-19)
4 files modified
src/maascli/__init__.py (+3/-11)
src/maascli/parser.py (+13/-1)
src/maascli/tests/test_integration.py (+10/-6)
src/maascli/tests/test_parser.py (+34/-1)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Alberto Donato (community) Approve
Review via email: mp+401465@code.launchpad.net

Commit message

Show best possible help for maas cli when missing arguments.

Signed-off-by: Adam Collard <email address hidden>

Description of the change

Before 1:
$ maas bolla
usage: maas [-h] COMMAND ...

optional arguments:
  -h, --help show this help message and exit

drill down:
  COMMAND
    login Log in to a remote API, and remember its description and credentials.
    logout Log out of a remote API, purging any stored credentials.
    list List remote APIs that have been logged-in to.
    refresh Refresh the API descriptions of all profiles.
    autopkgtest
                Interact with http://10.245.136.6:5240/MAAS/api/2.0/
    bolla Interact with http://bolla.internal:5240/MAAS/api/2.0/
    karura Interact with http://karura.internal:5240/MAAS/api/2.0/

http://maas.io/

too few arguments

After 1:
$ maas bolla
usage: maas bolla [-h] COMMAND ...

Issue commands to the MAAS region controller at http://bolla.internal:5240/MAAS/api/2.0/.

optional arguments:
  -h, --help show this help message and exit

drill down:
  COMMAND
    account Manage the current logged-in user.
    bcache-cache-set Manage bcache cache set on a machine.
    bcache-cache-sets Manage bcache cache sets on a machine.
    bcache Manage bcache device on a machine.
    bcaches Manage bcache devices on a machine.
    block-device Manage a block device on a machine.
    block-devices Manage block devices on a machine.
    boot-resource Manage a boot resource.
    boot-resources Manage the boot resources.
    boot-source Manage a boot source.
<snip>
Before 2:

$ maas bolla nodes
usage: maas [-h] COMMAND ...

optional arguments:
  -h, --help show this help message and exit

drill down:
  COMMAND
    login Log in to a remote API, and remember its description and credentials.
    logout Log out of a remote API, purging any stored credentials.
    list List remote APIs that have been logged-in to.
    refresh Refresh the API descriptions of all profiles.
    autopkgtest
                Interact with http://10.245.136.6:5240/MAAS/api/2.0/
    bolla Interact with http://bolla.internal:5240/MAAS/api/2.0/
    karura Interact with http://karura.internal:5240/MAAS/api/2.0/

http://maas.io/

too few arguments

After 2:
$ maas bolla nodes
usage: maas bolla nodes [-h] COMMAND ...

Manage the collection of all the nodes in the MAAS.

optional arguments:
  -h, --help show this help message and exit

drill down:
  COMMAND
    is-registered MAC address registered
    set-zone Assign nodes to a zone
    read List Nodes visible to the user
    is-action-in-progress
                        MAC address of deploying or commissioning node

too few arguments

To post a comment you must log in.
Revision history for this message
Alberto Donato (ack) wrote :

nice!

minor nits inline

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b maas-cli-tweaks lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 1afa53590b3f636d109f84b889e890dd778f7221

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b maas-cli-tweaks lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: d2d41a970bd71f60375bad34a00f701b07a6beac

review: Approve
Revision history for this message
Amy Pattanasethanon (amylily) wrote :

LGTM :shipit!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maascli/__init__.py b/src/maascli/__init__.py
2index cc1ca13..5a94ec0 100644
3--- a/src/maascli/__init__.py
4+++ b/src/maascli/__init__.py
5@@ -7,7 +7,7 @@
6 import os
7 import sys
8
9-from maascli.parser import prepare_parser
10+from maascli.parser import get_deepest_subparser, prepare_parser
11
12
13 def snap_setup():
14@@ -29,13 +29,6 @@ def main(argv=sys.argv):
15 # If no arguments have been passed be helpful and point out --help.
16 snap_setup()
17
18- if len(argv) == 1:
19- sys.stderr.write(
20- "Error: no arguments given.\n"
21- "Run %s --help for usage details.\n" % argv[0]
22- )
23- raise SystemExit(2)
24-
25 parser = prepare_parser(argv)
26
27 try:
28@@ -43,13 +36,12 @@ def main(argv=sys.argv):
29 if hasattr(options, "execute"):
30 options.execute(options)
31 else:
32+ sub_parser = get_deepest_subparser(parser, argv[1:])
33 # This mimics the error behaviour provided by argparse 1.1 from
34 # PyPI (which differs from argparse 1.1 in the standard library).
35- parser.error("too few arguments")
36+ sub_parser.error("too few arguments")
37 except KeyboardInterrupt:
38 raise SystemExit(1)
39- except SystemExit:
40- raise # Pass-through.
41 except Exception as error:
42 show = getattr(error, "always_show", False)
43 if options.debug or show:
44diff --git a/src/maascli/parser.py b/src/maascli/parser.py
45index 0e3cff0..8fe7a2d 100644
46--- a/src/maascli/parser.py
47+++ b/src/maascli/parser.py
48@@ -64,13 +64,25 @@ class ArgumentParser(argparse.ArgumentParser):
49 sys.exit(2)
50
51
52+def get_deepest_subparser(parser, argv):
53+ """Recursive function to find the best matching subparser."""
54+ if not argv:
55+ return parser
56+ maybe_parser, *rest = argv
57+ sub_parser = parser.subparsers._name_parser_map.get(maybe_parser)
58+ if sub_parser is None:
59+ return parser
60+ else:
61+ return get_deepest_subparser(sub_parser, rest)
62+
63+
64 def prepare_parser(argv):
65 """Create and populate an arguments parser for the maascli command."""
66 help_title, help_body = parse_docstring(api)
67 parser = ArgumentParser(
68 description=help_body,
69 prog=os.path.basename(argv[0]),
70- epilog="http://maas.io/",
71+ epilog="https://maas.io/",
72 )
73 register_cli_commands(parser)
74 api.register_api_commands(parser)
75diff --git a/src/maascli/tests/test_integration.py b/src/maascli/tests/test_integration.py
76index d455a06..01647e5 100644
77--- a/src/maascli/tests/test_integration.py
78+++ b/src/maascli/tests/test_integration.py
79@@ -29,13 +29,13 @@ class TestMAASCli(MAASTestCase):
80 self.assertRaises(CalledProcessError, self.run_command)
81
82 def test_run_without_args_shows_help_reminder(self):
83- self.output_file = self.make_file("output")
84 try:
85 self.run_command()
86 except CalledProcessError as error:
87- self.assertIn(
88- "Run %s --help for usage details." % locate_maascli(),
89- error.output.decode("ascii"),
90+ self.assertTrue(
91+ error.output.decode("utf-8").startswith(
92+ "usage: maas [-h] COMMAND"
93+ )
94 )
95
96 def test_help_option_succeeds(self):
97@@ -71,12 +71,16 @@ class TestMain(MAASTestCase):
98 [profile_name] = configs
99 resources = configs[profile_name]["description"]["resources"]
100 resource_name = random.choice(resources)["name"]
101- command = "maas", profile_name, handler_command_name(resource_name)
102+ handler_name = handler_command_name(resource_name)
103+ command = "maas", profile_name, handler_name
104
105 with CaptureStandardIO() as stdio:
106 error = self.assertRaises(SystemExit, main, command)
107
108 self.assertEqual(error.code, 2)
109 error = stdio.getError()
110- self.assertIn("usage: maas [-h] COMMAND ...", error)
111+ self.assertIn(
112+ f"usage: maas {profile_name} {handler_name} [-h] COMMAND ...",
113+ error,
114+ )
115 self.assertIn("too few arguments", error)
116diff --git a/src/maascli/tests/test_parser.py b/src/maascli/tests/test_parser.py
117index 203d438..e8322b0 100644
118--- a/src/maascli/tests/test_parser.py
119+++ b/src/maascli/tests/test_parser.py
120@@ -3,7 +3,11 @@
121
122 import sys
123
124-from maascli.parser import ArgumentParser, prepare_parser
125+from maascli.parser import (
126+ ArgumentParser,
127+ get_deepest_subparser,
128+ prepare_parser,
129+)
130 from maastesting.factory import factory
131 from maastesting.testcase import MAASTestCase
132
133@@ -67,3 +71,32 @@ class TestArgumentParser(MAASTestCase):
134 except TypeError:
135 pass
136 mock_exit.assert_called_once_with(2)
137+
138+
139+class TestGetDeepestSubparser(MAASTestCase):
140+ def test_no_argv(self):
141+ parser = ArgumentParser()
142+ assert get_deepest_subparser(parser, []) is parser
143+
144+ def test_single_subparser(self):
145+ top_parser = ArgumentParser()
146+ foo = top_parser.subparsers.add_parser("foo", help="foo help")
147+
148+ assert get_deepest_subparser(top_parser, ["foo"]) is foo
149+
150+ def test_nested_subparser(self):
151+ top_parser = ArgumentParser()
152+ foo = top_parser.subparsers.add_parser("foo", help="foo help")
153+ bar = foo.subparsers.add_parser("bar", help="bar help")
154+
155+ assert get_deepest_subparser(top_parser, ["foo", "bar"]) is bar
156+
157+ def test_not_a_subparser(self):
158+ top_parser = ArgumentParser()
159+ foo = top_parser.subparsers.add_parser("foo", help="foo help")
160+ bar = foo.subparsers.add_parser("bar", help="bar help")
161+
162+ assert (
163+ get_deepest_subparser(top_parser, ["foo", "bar", "--help"]) is bar
164+ )
165+ assert get_deepest_subparser(top_parser, ["--random"]) is top_parser

Subscribers

People subscribed via source and target branches