launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13141
[Merge] lp:~jtv/maas/maascli-cli-subcommand into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/maascli-cli-subcommand into lp:maas.
Commit message:
Move maascli's CLI management commands (login, list etc.) into a dedicated sub-command "cli."
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/maascli-cli-subcommand/+merge/128538
This was pre-imped ages ago, I think several times actually. It seems like a complication to say "maascli cli list" instead of just "maascli list," and so on, but after this I'll be able to make maascli pick a default profile, and move the current profile's API operations directly into the main namespace. So in return for this extra work, you'll be able to type API verbs directly after the "maascli" command.
Jeroen
--
https://code.launchpad.net/~jtv/maas/maascli-cli-subcommand/+merge/128538
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/maascli-cli-subcommand into lp:maas.
=== modified file 'src/maascli/__init__.py'
--- src/maascli/__init__.py 2012-10-08 08:55:15 +0000
+++ src/maascli/__init__.py 2012-10-08 16:13:36 +0000
@@ -14,41 +14,11 @@
"main",
]
-import argparse
-from argparse import RawDescriptionHelpFormatter
import locale
import sys
from bzrlib import osutils
-from maascli.api import register_api_commands
-from maascli.cli import register_cli_commands
-from maascli.utils import parse_docstring
-
-
-class ArgumentParser(argparse.ArgumentParser):
- """Specialisation of argparse's parser with better support for subparsers.
-
- Specifically, the one-shot `add_subparsers` call is disabled, replaced by
- a lazily evaluated `subparsers` property.
- """
-
- def __init__(self, *args, **kwargs):
- kwargs.setdefault("formatter_class", RawDescriptionHelpFormatter)
- super(ArgumentParser, self).__init__(*args, **kwargs)
-
- def add_subparsers(self):
- raise NotImplementedError(
- "add_subparsers has been disabled")
-
- @property
- def subparsers(self):
- try:
- return self.__subparsers
- except AttributeError:
- parent = super(ArgumentParser, self)
- self.__subparsers = parent.add_subparsers(title="drill down")
- self.__subparsers.metavar = "COMMAND"
- return self.__subparsers
+from maascli.parser import prepare_parser
def main(argv=None):
@@ -58,13 +28,7 @@
if argv is None:
argv = sys.argv[:1] + osutils.get_unicode_argv()
- module = __import__('maascli.api', fromlist=True)
- help_title, help_body = parse_docstring(module)
- parser = ArgumentParser(
- description=help_body, prog=argv[0],
- epilog="http://maas.ubuntu.com/";)
- register_cli_commands(parser)
- register_api_commands(parser)
+ parser = prepare_parser(argv)
# Run, doing polite things with exceptions.
try:
=== modified file 'src/maascli/cli.py'
--- src/maascli/cli.py 2012-10-08 13:39:12 +0000
+++ src/maascli/cli.py 2012-10-08 16:13:36 +0000
@@ -129,8 +129,10 @@
def register_cli_commands(parser):
"""Register the CLI's meta-subcommands on `parser`."""
+ cli_parser = parser.subparsers.add_parser(
+ 'cli', help="CLI management commands")
for name, command in commands.items():
help_title, help_body = parse_docstring(command)
- command_parser = parser.subparsers.add_parser(
+ command_parser = cli_parser.subparsers.add_parser(
safe_name(name), help=help_title, description=help_body)
command_parser.set_defaults(execute=command(command_parser))
=== added file 'src/maascli/parser.py'
--- src/maascli/parser.py 1970-01-01 00:00:00 +0000
+++ src/maascli/parser.py 2012-10-08 16:13:36 +0000
@@ -0,0 +1,72 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Arguments parser for `maascli`."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = [
+ 'prepare_parser',
+ ]
+
+import argparse
+
+from maascli import api
+from maascli.cli import register_cli_commands
+from maascli.utils import parse_docstring
+
+
+class ArgumentParser(argparse.ArgumentParser):
+ """Specialisation of argparse's parser with better support for subparsers.
+
+ Specifically, the one-shot `add_subparsers` call is disabled, replaced by
+ a lazily evaluated `subparsers` property.
+ """
+
+ def __init__(self, *args, **kwargs):
+ kwargs.setdefault(
+ "formatter_class", argparse.RawDescriptionHelpFormatter)
+ super(ArgumentParser, self).__init__(*args, **kwargs)
+
+ def add_subparsers(self):
+ raise NotImplementedError(
+ "add_subparsers has been disabled")
+
+ @property
+ def subparsers(self):
+ try:
+ return self.__subparsers
+ except AttributeError:
+ parent = super(ArgumentParser, self)
+ self.__subparsers = parent.add_subparsers(title="drill down")
+ self.__subparsers.metavar = "COMMAND"
+ return self.__subparsers
+
+
+def get_profile_option(argv):
+ """Parse the `--profile` option in `argv`; ignore the rest."""
+ # Create a specialized parser just to extract this one option.
+ # If we call parse_known_args on the real arguments parser, the
+ # --help option will do its work and cause the process to exit
+ # before we can even add the sub-parsers that the user may be asking
+ # for help about.
+ specialized_parser = ArgumentParser(add_help=False)
+ specialized_parser.add_argument('--profile', metavar='PROFILE')
+ provisional_options = specialized_parser.parse_known_args(argv)[0]
+ return provisional_options.profile
+
+
+def prepare_parser(argv):
+ """Create and populate an arguments parser for the maascli command."""
+ help_title, help_body = parse_docstring(api)
+ parser = ArgumentParser(
+ description=help_body, prog=argv[0],
+ epilog="http://maas.ubuntu.com/";)
+ register_cli_commands(parser)
+ api.register_api_commands(parser)
+ return parser
=== modified file 'src/maascli/tests/test_api.py'
--- src/maascli/tests/test_api.py 2012-10-08 13:30:28 +0000
+++ src/maascli/tests/test_api.py 2012-10-08 16:13:36 +0000
@@ -18,12 +18,10 @@
import json
import httplib2
-from maascli import (
- api,
- ArgumentParser,
- )
+from maascli import api
from maascli.command import CommandError
from maascli.config import ProfileConfig
+from maascli.parser import ArgumentParser
from maascli.testing.config import make_configs
from maascli.utils import (
handler_command_name,
=== modified file 'src/maascli/tests/test_cli.py'
--- src/maascli/tests/test_cli.py 2012-10-08 06:30:25 +0000
+++ src/maascli/tests/test_cli.py 2012-10-08 16:13:36 +0000
@@ -12,10 +12,8 @@
__metaclass__ = type
__all__ = []
-from maascli import (
- ArgumentParser,
- cli,
- )
+from maascli import cli
+from maascli.parser import ArgumentParser
from maastesting.testcase import TestCase
@@ -31,6 +29,7 @@
def test_subparsers_have_appropriate_execute_defaults(self):
parser = ArgumentParser()
cli.register_cli_commands(parser)
+ cli_parser = parser.subparsers.choices['cli']
self.assertIsInstance(
- parser.subparsers.choices['login'].get_default('execute'),
+ cli_parser.subparsers.choices['login'].get_default('execute'),
cli.cmd_login)
=== modified file 'src/maascli/tests/test_integration.py'
--- src/maascli/tests/test_integration.py 2012-10-04 08:36:34 +0000
+++ src/maascli/tests/test_integration.py 2012-10-08 16:13:36 +0000
@@ -48,6 +48,6 @@
pass
def test_list_command_succeeds(self):
- self.run_command('list')
+ self.run_command('cli', 'list')
# The test is that we get here without error.
pass
=== renamed file 'src/maascli/tests/test_init.py' => 'src/maascli/tests/test_parser.py'
--- src/maascli/tests/test_init.py 2012-10-05 05:12:01 +0000
+++ src/maascli/tests/test_parser.py 2012-10-08 16:13:36 +0000
@@ -12,11 +12,16 @@
__metaclass__ = type
__all__ = []
-from maascli import ArgumentParser
+from maascli.parser import (
+ ArgumentParser,
+ get_profile_option,
+ )
+from maastesting.factory import factory
from maastesting.testcase import TestCase
class TestArgumentParser(TestCase):
+ """Tests for `ArgumentParser`."""
def test_add_subparsers_disabled(self):
parser = ArgumentParser()
@@ -36,3 +41,29 @@
# The subparsers property, once populated, always returns the same
# object.
self.assertIs(subparsers, parser.subparsers)
+
+
+class TestGetProfileOption(TestCase):
+ """Tests for `get_profile_option`."""
+
+ def test_parses_profile_option(self):
+ profile = factory.make_name('profile')
+ self.assertEqual(profile, get_profile_option(['--profile', profile]))
+
+ def test_ignores_other_options(self):
+ profile = factory.make_name('profile')
+ self.assertEqual(
+ profile,
+ get_profile_option([
+ '--unrelated', 'option',
+ '--profile', profile,
+ factory.getRandomString(),
+ ]))
+
+ def test_ignores_help_option(self):
+ # This is a bit iffy: the most likely symptom if this fails is
+ # actually that the test process exits!
+ profile = factory.make_name('profile')
+ self.assertEqual(
+ profile,
+ get_profile_option(['--help', '--profile', profile]))
