Merge lp:~jtv/maas/bug-1058282-ditch-api into lp:maas/trunk

Proposed by Jeroen T. Vermeulen on 2012-10-04
Status: Merged
Approved by: Jeroen T. Vermeulen on 2012-10-04
Approved revision: 1161
Merged at revision: 1168
Proposed branch: lp:~jtv/maas/bug-1058282-ditch-api
Merge into: lp:maas/trunk
Diff against target: 98 lines (+58/-17)
2 files modified
src/maascli/__init__.py (+5/-17)
src/maascli/tests/test_integration.py (+53/-0)
To merge this branch: bzr merge lp:~jtv/maas/bug-1058282-ditch-api
Reviewer Review Type Date Requested Status
John A Meinel Approve on 2012-10-04
Jeroen T. Vermeulen (community) Abstain on 2012-10-04
Review via email: mp+127959@code.launchpad.net

Commit Message

Ditch the "api" module identifier on maascli command lines. It's the only module we have right now, and in a future where we have others, it's still the sensible default.

Description of the Change

Pre-imped with Gavin. This shortens all maascli command lines by one word: not enough to make it properly user-friendly, but it's one step in the right direction.

You'll note that there was no integration test for this change to break. So I added one. Not quite detailed enough that it would have noticed the change, but enough that it would have noticed if I broke the command entirely.

Jeroen

To post a comment you must log in.
Jeroen T. Vermeulen (jtv) :
review: Approve
Jeroen T. Vermeulen (jtv) wrote :

Oops. Accidentally voted Approve.

review: Abstain
John A Meinel (jameinel) wrote :

You've removed the ability to add modules dynamically (via bootstrapping your model into the dict), but it probably isn't a big deal.

I'm a little concerned about namespacing errors as we move forward, which I'm sure is why Gavin did it in the first place.

But I agree that it is nice to be less wordy in getting things done (bin/maascli api maas tags list is a bit longwinded)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maascli/__init__.py'
2--- src/maascli/__init__.py 2012-09-18 16:45:00 +0000
3+++ src/maascli/__init__.py 2012-10-04 08:39:29 +0000
4@@ -31,11 +31,6 @@
5 )
6
7
8-modules = {
9- "api": "maascli.api",
10- }
11-
12-
13 class ArgumentParser(argparse.ArgumentParser):
14 """Specialisation of argparse's parser with better support for subparsers.
15
16@@ -64,19 +59,12 @@
17 if argv is None:
18 argv = sys.argv[:1] + osutils.get_unicode_argv()
19
20- # Create the base argument parser.
21+ module = __import__('maascli.api', fromlist=True)
22+ help_title, help_body = parse_docstring(module)
23 parser = ArgumentParser(
24- description="Control MAAS from the command-line.",
25- prog=argv[0], epilog="http://maas.ubuntu.com/")
26-
27- # Register declared modules.
28- for name, module in sorted(modules.items()):
29- if isinstance(module, basestring):
30- module = __import__(module, fromlist=True)
31- help_title, help_body = parse_docstring(module)
32- module_parser = parser.subparsers.add_parser(
33- name, help=help_title, description=help_body)
34- register(module, module_parser)
35+ description=help_body, prog=argv[0],
36+ epilog="http://maas.ubuntu.com/")
37+ register(module, parser)
38
39 # Run, doing polite things with exceptions.
40 try:
41
42=== added file 'src/maascli/tests/test_integration.py'
43--- src/maascli/tests/test_integration.py 1970-01-01 00:00:00 +0000
44+++ src/maascli/tests/test_integration.py 2012-10-04 08:39:29 +0000
45@@ -0,0 +1,53 @@
46+# Copyright 2012 Canonical Ltd. This software is licensed under the
47+# GNU Affero General Public License version 3 (see the file LICENSE).
48+
49+"""Integration-test the `maascli` command."""
50+
51+from __future__ import (
52+ absolute_import,
53+ print_function,
54+ unicode_literals,
55+ )
56+
57+__metaclass__ = type
58+__all__ = []
59+
60+import os.path
61+from subprocess import (
62+ CalledProcessError,
63+ check_call,
64+ )
65+
66+from maastesting.testcase import TestCase
67+
68+
69+def locate_dev_root():
70+ """Root of development tree that this test is in."""
71+ return os.path.join(
72+ os.path.dirname(__file__), os.pardir, os.pardir, os.pardir)
73+
74+
75+def locate_maascli():
76+ return os.path.join(locate_dev_root(), 'bin', 'maascli')
77+
78+
79+class TestMAASCli(TestCase):
80+
81+ def run_command(self, *args):
82+ with open('/dev/null', 'ab') as dev_null:
83+ check_call(
84+ [locate_maascli()] + list(args),
85+ stdout=dev_null, stderr=dev_null)
86+
87+ def test_run_without_args_fails(self):
88+ self.assertRaises(CalledProcessError, self.run_command)
89+
90+ def test_help_option_succeeds(self):
91+ self.run_command('-h')
92+ # The test is that we get here without error.
93+ pass
94+
95+ def test_list_command_succeeds(self):
96+ self.run_command('list')
97+ # The test is that we get here without error.
98+ pass