launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28568
[Merge] ~cjwatson/lpcraft:craft-cli-dispatcher into lpcraft:main
Colin Watson has proposed merging ~cjwatson/lpcraft:craft-cli-dispatcher into lpcraft:main.
Commit message:
Use the craft-cli command dispatcher
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/lpcraft/+git/lpcraft/+merge/424297
The main motivation for this is that it moves per-command argument handling to be alongside the implementation of the command. It also makes it easier to get rid of several fragile `getattr` calls.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lpcraft:craft-cli-dispatcher into lpcraft:main.
diff --git a/NEWS.rst b/NEWS.rst
index da6454b..6e552fb 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -7,6 +7,8 @@ Version history
- Rewrite the release documentation.
+- Use the ``craft-cli`` command dispatcher.
+
0.0.15 (2022-06-01)
===================
diff --git a/lpcraft/commands/clean.py b/lpcraft/commands/clean.py
index 5d67579..95a19b4 100644
--- a/lpcraft/commands/clean.py
+++ b/lpcraft/commands/clean.py
@@ -1,34 +1,51 @@
# Copyright 2022 Canonical Ltd. This software is licensed under the
# GNU General Public License version 3 (see the file LICENSE).
-from argparse import Namespace
+from argparse import ArgumentParser, Namespace
from pathlib import Path
-from craft_cli import emit
+from craft_cli import BaseCommand, emit
from lpcraft.config import Config
from lpcraft.providers import get_provider
-def clean(args: Namespace) -> int:
+class CleanCommand(BaseCommand):
"""Clean the managed environments for a project."""
- # We want to run the "clean" command only when run from
- # an lpcraft project directory.
- config_path = getattr(args, "config", Path(".launchpad.yaml"))
- Config.load(config_path)
-
- cwd = Path.cwd()
- emit.progress(
- f"Deleting the managed environments for the {cwd.name!r} project."
- )
-
- provider = get_provider()
- provider.ensure_provider_is_available()
-
- provider.clean_project_environments(
- project_name=cwd.name, project_path=cwd
- )
- emit.message(
- f"Deleted the managed environments for the {cwd.name!r} project."
- )
- return 0
+
+ name = "clean"
+ help_msg = __doc__.splitlines()[0]
+ overview = __doc__
+ common = True
+
+ def fill_parser(self, parser: ArgumentParser) -> None:
+ """Add arguments specific to this command."""
+ parser.add_argument(
+ "-c",
+ "--config",
+ type=Path,
+ default=".launchpad.yaml",
+ help="Read the configuration file from this path.",
+ )
+
+ def run(self, args: Namespace) -> int:
+ """Run the command."""
+ # We want to run the "clean" command only when run from
+ # an lpcraft project directory.
+ Config.load(args.config)
+
+ cwd = Path.cwd()
+ emit.progress(
+ f"Deleting the managed environments for the {cwd.name!r} project."
+ )
+
+ provider = get_provider()
+ provider.ensure_provider_is_available()
+
+ provider.clean_project_environments(
+ project_name=cwd.name, project_path=cwd
+ )
+ emit.message(
+ f"Deleted the managed environments for the {cwd.name!r} project."
+ )
+ return 0
diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py
index 4a0ab82..0175b00 100644
--- a/lpcraft/commands/run.py
+++ b/lpcraft/commands/run.py
@@ -7,11 +7,11 @@ import itertools
import json
import os
import shlex
-from argparse import Namespace
+from argparse import ArgumentParser, Namespace
from pathlib import Path, PurePath
from typing import Dict, List, Optional, Set
-from craft_cli import EmitterMode, emit
+from craft_cli import BaseCommand, EmitterMode, emit
from craft_providers import Executor, lxd
from craft_providers.actions.snap_installer import install_from_store
from dotenv import dotenv_values
@@ -427,105 +427,187 @@ def _get_job_instance_name(provider: Provider, job: Job) -> str:
)
-def run(args: Namespace) -> int:
+class RunCommand(BaseCommand):
"""Run a pipeline, launching managed environments as needed."""
- # XXX jugmac00 2022-02-04: this fallback may become obsolete once we
- # use craft-cli's command dispatcher
- config_path = getattr(args, "config", Path(".launchpad.yaml"))
- config = Config.load(config_path)
- provider = get_provider()
- provider.ensure_provider_is_available()
- launched_instances = []
+ name = "run"
+ help_msg = __doc__.splitlines()[0]
+ overview = __doc__
+ common = True
+
+ def fill_parser(self, parser: ArgumentParser) -> None:
+ """Add arguments specific to this command."""
+ parser.add_argument(
+ "--output-directory",
+ type=Path,
+ help="Write output files to this directory.",
+ )
+ parser.add_argument(
+ "-c",
+ "--config",
+ type=Path,
+ default=".launchpad.yaml",
+ help="Read the configuration file from this path.",
+ )
+ parser.add_argument(
+ "--clean",
+ action="store_true",
+ default=False,
+ help=(
+ "Clean the managed environments created "
+ "for the pipeline after the running it."
+ ),
+ )
+ parser.add_argument(
+ "--apt-replace-repositories",
+ action="append",
+ help="Overwrite /etc/apt/sources.list.",
+ )
+ parser.add_argument(
+ "--set-env",
+ action="append",
+ help="Set an environment variable.",
+ )
- try:
- for stage in config.pipeline:
- stage_failed = False
- for job_name in stage:
- try:
- jobs = config.jobs.get(job_name, [])
- if not jobs:
- raise CommandError(
- f"No job definition for {job_name!r}"
- )
- for job in jobs:
- launched_instances.append(
- _get_job_instance_name(provider, job)
- )
- _run_job(
- job_name,
- job,
- provider,
- getattr(args, "output_directory", None),
- apt_replacement_repositories=getattr(
- args, "apt_replace_repositories", None
- ),
- env_from_cli=getattr(args, "set_env", None),
- )
- except CommandError as e:
- if len(stage) == 1:
- # Single-job stage, so just reraise this
- # in order to get simpler error messages.
- raise
- else:
- emit.error(e)
- stage_failed = True
- if stage_failed:
- raise CommandError(
- f"Some jobs in {stage} failed; stopping.", retcode=1
+ def run(self, args: Namespace) -> int:
+ """Run the command."""
+ config = Config.load(args.config)
+
+ provider = get_provider()
+ provider.ensure_provider_is_available()
+ launched_instances = []
+
+ try:
+ for stage in config.pipeline:
+ stage_failed = False
+ for job_name in stage:
+ try:
+ jobs = config.jobs.get(job_name, [])
+ if not jobs:
+ raise CommandError(
+ f"No job definition for {job_name!r}"
+ )
+ for job in jobs:
+ launched_instances.append(
+ _get_job_instance_name(provider, job)
+ )
+ _run_job(
+ job_name,
+ job,
+ provider,
+ args.output_directory,
+ apt_replacement_repositories=(
+ args.apt_replace_repositories
+ ),
+ env_from_cli=args.set_env,
+ )
+ except CommandError as e:
+ if len(stage) == 1:
+ # Single-job stage, so just reraise this
+ # in order to get simpler error messages.
+ raise
+ else:
+ emit.error(e)
+ stage_failed = True
+ if stage_failed:
+ raise CommandError(
+ f"Some jobs in {stage} failed; stopping.", retcode=1
+ )
+ finally:
+ if args.clean:
+ cwd = Path.cwd()
+ provider.clean_project_environments(
+ project_name=cwd.name,
+ project_path=cwd,
+ instances=launched_instances,
)
- finally:
- should_clean_environment = getattr(args, "clean", False)
-
- if should_clean_environment:
- cwd = Path.cwd()
- provider.clean_project_environments(
- project_name=cwd.name,
- project_path=cwd,
- instances=launched_instances,
- )
- return 0
+ return 0
-def run_one(args: Namespace) -> int:
+class RunOneCommand(BaseCommand):
"""Select and run a single job from a pipeline.
(This command is for use by Launchpad, and is subject to change.)
"""
- config = Config.load(args.config)
- jobs = config.jobs.get(args.job, [])
- if not jobs:
- raise CommandError(f"No job definition for {args.job!r}")
- if args.index >= len(jobs):
- raise CommandError(
- f"No job definition with index {args.index} for {args.job!r}"
+ name = "run-one"
+ help_msg = __doc__.splitlines()[0]
+ overview = __doc__
+ hidden = True
+
+ def fill_parser(self, parser: ArgumentParser) -> None:
+ """Add arguments specific to this command."""
+ parser.add_argument(
+ "--output-directory",
+ type=Path,
+ help="Write output files to this directory.",
)
- job = jobs[args.index]
-
- provider = get_provider()
- provider.ensure_provider_is_available()
-
- try:
- _run_job(
- args.job,
- job,
- provider,
- getattr(args, "output_directory", None),
- apt_replacement_repositories=getattr(
- args, "apt_replace_repositories", None
+ parser.add_argument(
+ "-c",
+ "--config",
+ type=Path,
+ default=".launchpad.yaml",
+ help="Read the configuration file from this path.",
+ )
+ parser.add_argument(
+ "--clean",
+ action="store_true",
+ default=False,
+ help=(
+ "Clean the managed environment created for the job "
+ "after running it."
),
- env_from_cli=getattr(args, "set_env", None),
)
- finally:
- should_clean_environment = getattr(args, "clean", False)
-
- if should_clean_environment:
- cwd = Path.cwd()
- provider.clean_project_environments(
- project_name=cwd.name,
- project_path=cwd,
- instances=[_get_job_instance_name(provider, job)],
+ parser.add_argument("job", help="Run only this job name.")
+ parser.add_argument(
+ "index",
+ type=int,
+ metavar="N",
+ help="Run only the Nth job with the given name (indexing from 0).",
+ )
+ parser.add_argument(
+ "--apt-replace-repositories",
+ action="append",
+ help="Overwrite /etc/apt/sources.list.",
+ )
+ parser.add_argument(
+ "--set-env",
+ action="append",
+ help="Set an environment variable.",
+ )
+
+ def run(self, args: Namespace) -> int:
+ """Run the command."""
+ config = Config.load(args.config)
+
+ jobs = config.jobs.get(args.job, [])
+ if not jobs:
+ raise CommandError(f"No job definition for {args.job!r}")
+ if args.index >= len(jobs):
+ raise CommandError(
+ f"No job definition with index {args.index} for {args.job!r}"
+ )
+ job = jobs[args.index]
+
+ provider = get_provider()
+ provider.ensure_provider_is_available()
+
+ try:
+ _run_job(
+ args.job,
+ job,
+ provider,
+ args.output_directory,
+ apt_replacement_repositories=args.apt_replace_repositories,
+ env_from_cli=args.set_env,
)
+ finally:
+ if args.clean:
+ cwd = Path.cwd()
+ provider.clean_project_environments(
+ project_name=cwd.name,
+ project_path=cwd,
+ instances=[_get_job_instance_name(provider, job)],
+ )
- return 0
+ return 0
diff --git a/lpcraft/commands/version.py b/lpcraft/commands/version.py
index 40f7ce3..94acff7 100644
--- a/lpcraft/commands/version.py
+++ b/lpcraft/commands/version.py
@@ -3,12 +3,20 @@
from argparse import Namespace
-from craft_cli import emit
+from craft_cli import BaseCommand, emit
from lpcraft._version import version_description as lpcraft_version
-def version(args: Namespace) -> int:
+class VersionCommand(BaseCommand):
"""Show lpcraft's version number."""
- emit.message(lpcraft_version)
- return 0
+
+ name = "version"
+ help_msg = __doc__.splitlines()[0]
+ overview = __doc__
+ common = True
+
+ def run(self, args: Namespace) -> int:
+ """Run the command."""
+ emit.message(lpcraft_version)
+ return 0
diff --git a/lpcraft/main.py b/lpcraft/main.py
index c9af819..83fe767 100644
--- a/lpcraft/main.py
+++ b/lpcraft/main.py
@@ -4,17 +4,24 @@
"""Main entry point."""
import logging
-from argparse import ArgumentParser
-from pathlib import Path
+import sys
from typing import List, Optional
-from craft_cli import CraftError, EmitterMode, emit
+from craft_cli import (
+ ArgumentParsingError,
+ CommandGroup,
+ CraftError,
+ Dispatcher,
+ EmitterMode,
+ GlobalArgument,
+ ProvideHelpException,
+ emit,
+)
from lpcraft._version import version_description as lpcraft_version
-from lpcraft.commands.clean import clean
-from lpcraft.commands.run import run, run_one
-from lpcraft.commands.version import version
-from lpcraft.errors import CommandError
+from lpcraft.commands.clean import CleanCommand
+from lpcraft.commands.run import RunCommand, RunOneCommand
+from lpcraft.commands.version import VersionCommand
def _configure_logger(name: str) -> None:
@@ -30,155 +37,56 @@ def _configure_logger(name: str) -> None:
_configure_logger("craft_providers")
+_basic_commands = [
+ CleanCommand,
+ RunCommand,
+ RunOneCommand,
+ VersionCommand,
+]
+
+
def main(argv: Optional[List[str]] = None) -> int:
"""`lpcraft` runs Launchpad CI jobs."""
- parser = ArgumentParser(description="Run Launchpad CI jobs.")
- parser.add_argument(
- "--version",
- action="store_true",
- help="Show version information and exit.",
- )
-
- verbosity_group = parser.add_mutually_exclusive_group()
- verbosity_group.add_argument(
- "-q",
- "--quiet",
- action="store_true",
- help="Only show warnings and errors, not progress.",
- )
- verbosity_group.add_argument(
- "-v",
- "--verbose",
- action="store_true",
- help="Show debug information and be more verbose.",
- )
- verbosity_group.add_argument(
- "--trace",
- action="store_true",
- help="Show all information needed to trace internal behaviour.",
- )
-
- subparsers = parser.add_subparsers()
-
- # XXX cjwatson 2021-11-15: Subcommand arguments should be defined
- # alongside the individual subcommands rather than here.
- parser_clean = subparsers.add_parser(
- "clean", description=clean.__doc__, help=clean.__doc__
- )
- parser_clean.add_argument(
- "-c",
- "--config",
- type=Path,
- default=".launchpad.yaml",
- help="Read the configuration file from this path.",
- )
- parser_clean.set_defaults(func=clean)
-
- parser_run = subparsers.add_parser(
- "run", description=run.__doc__, help=run.__doc__
- )
- parser_run.add_argument(
- "--output-directory",
- type=Path,
- help="Write output files to this directory.",
- )
- parser_run.add_argument(
- "-c",
- "--config",
- type=Path,
- default=".launchpad.yaml",
- help="Read the configuration file from this path.",
- )
- parser_run.add_argument(
- "--clean",
- action="store_true",
- help=(
- "Clean the managed environments created "
- "for the pipeline after the running it."
- ),
- )
- parser_run.add_argument(
- "--apt-replace-repositories",
- action="append",
- help="Overwrite /etc/apt/sources.list.",
- )
- parser_run.add_argument(
- "--set-env",
- action="append",
- help="Set an environment variable.",
- )
- parser_run.set_defaults(func=run)
-
- parser_run_one = subparsers.add_parser(
- "run-one", description=run_one.__doc__, help=run_one.__doc__
- )
- parser_run_one.add_argument(
- "--output-directory",
- type=Path,
- help="Write output files to this directory.",
- )
- parser_run_one.add_argument(
- "-c",
- "--config",
- type=Path,
- default=".launchpad.yaml",
- help="Read the configuration file from this path.",
- )
- parser_run_one.add_argument(
- "--clean",
- action="store_true",
- help=(
- "Clean the managed environment created for the job "
- "after running it."
- ),
- )
- parser_run_one.add_argument("job", help="Run only this job name.")
- parser_run_one.add_argument(
- "index",
- type=int,
- metavar="N",
- help="Run only the Nth job with the given name (indexing from 0).",
- )
- parser_run_one.add_argument(
- "--apt-replace-repositories",
- action="append",
- help="Overwrite /etc/apt/sources.list.",
- )
- parser_run_one.add_argument(
- "--set-env",
- action="append",
- help="Set an environment variable.",
- )
- parser_run_one.set_defaults(func=run_one)
-
- parser_version = subparsers.add_parser(
- "version", description=version.__doc__, help=version.__doc__
- )
- parser_version.set_defaults(func=version)
+ if argv is None:
+ argv = sys.argv[1:]
emit.init(EmitterMode.NORMAL, "lpcraft", f"Starting {lpcraft_version}")
+ command_groups = [CommandGroup("Basic", _basic_commands)]
+ summary = "Run Launchpad CI jobs."
+ extra_global_args = [
+ GlobalArgument(
+ "version",
+ "flag",
+ "-V",
+ "--version",
+ "Show version information and exit",
+ )
+ ]
try:
- args = parser.parse_args(argv)
- except SystemExit:
+ dispatcher = Dispatcher(
+ "lpcraft",
+ command_groups,
+ summary=summary,
+ extra_global_args=extra_global_args,
+ default_command=RunCommand,
+ )
+ global_args = dispatcher.pre_parse_args(argv)
+ if global_args["version"]:
+ emit.message(lpcraft_version)
+ emit.ended_ok()
+ return 0
+ dispatcher.load_command(None)
+ ret = dispatcher.run() or 0
+ except ArgumentParsingError as e:
+ print(e, file=sys.stderr)
emit.ended_ok()
- return 1
-
- if args.quiet:
- emit.set_mode(EmitterMode.QUIET)
- elif args.verbose:
- emit.set_mode(EmitterMode.VERBOSE)
- elif args.trace:
- emit.set_mode(EmitterMode.TRACE)
-
- if args.version:
- emit.message(lpcraft_version)
+ ret = 1
+ except ProvideHelpException as e:
+ print(e)
emit.ended_ok()
- return 0
-
- try:
- ret = int(getattr(args, "func", run)(args))
- except CommandError as e:
+ ret = 0
+ except CraftError as e:
emit.error(e)
ret = e.retcode
except KeyboardInterrupt as e:
diff --git a/lpcraft/tests/test_main.py b/lpcraft/tests/test_main.py
index 423d7ec..85aae92 100644
--- a/lpcraft/tests/test_main.py
+++ b/lpcraft/tests/test_main.py
@@ -1,6 +1,7 @@
# Copyright 2021 Canonical Ltd. This software is licensed under the
# GNU General Public License version 3 (see the file LICENSE).
+import io
from unittest.mock import call, patch
from craft_cli import CraftError, EmitterMode
@@ -26,38 +27,44 @@ class TestMain(TestCase):
mock_emit.message.assert_called_once_with(lpcraft_version)
mock_emit.ended_ok.assert_called_once_with()
- def test_bad_arguments(self):
+ @patch("sys.stderr", new_callable=io.StringIO)
+ def test_bad_arguments(self, mock_stderr):
# main() exits appropriately if given bad arguments.
mock_emit = self.useFixture(MockPatch("lpcraft.main.emit")).mock
- mock_argparse_print_message = self.useFixture(
- MockPatch("argparse.ArgumentParser._print_message")
- ).mock
ret = main(["--nonexistent"])
self.assertEqual(1, ret)
- # using `assert_called_with` is not possible as the message is
- # different depending whether pytest or coverage is driving the tests
self.assertIn(
- "error: unrecognized arguments: --nonexistent\n",
- mock_argparse_print_message.call_args.args[0],
+ "Error: unrecognized arguments: --nonexistent\n",
+ mock_stderr.getvalue(),
)
mock_emit.ended_ok.assert_called_once_with()
- @patch("lpcraft.main.run")
+ @patch("sys.stdout", new_callable=io.StringIO)
+ def test_help(self, mock_stdout):
+ mock_emit = self.useFixture(MockPatch("lpcraft.main.emit")).mock
+
+ ret = main(["--help"])
+
+ self.assertEqual(0, ret)
+ self.assertIn("Usage:\n", mock_stdout.getvalue())
+ mock_emit.ended_ok.assert_called_once_with()
+
+ @patch("lpcraft.commands.run.RunCommand.run")
def test_keyboard_interrupt(self, mock_run):
mock_run.side_effect = KeyboardInterrupt()
with RecordingEmitterFixture() as emitter:
- ret = main()
+ ret = main([])
self.assertEqual(1, ret)
self.assertEqual(
call("error", CraftError("Interrupted.")),
- emitter.recorder.interactions[0],
+ emitter.recorder.interactions[-1],
)
- @patch("lpcraft.main.run")
+ @patch("lpcraft.commands.run.RunCommand.run")
def test_handling_unexpected_exception(self, mock_run):
self.useFixture(MockPatch("sys.argv", ["lpcraft"]))
mock_run.side_effect = RuntimeError()
@@ -70,7 +77,7 @@ class TestMain(TestCase):
call(
"error", CraftError("lpcraft internal error: RuntimeError()")
),
- emitter.recorder.interactions[0],
+ emitter.recorder.interactions[-1],
)
def test_quiet_mode(self):
@@ -83,7 +90,7 @@ class TestMain(TestCase):
# result is something like "lpcraft, version 0.0.1"
self.assertIn(
- "lpcraft, version", emitter.recorder.interactions[0].args[1]
+ "lpcraft, version", emitter.recorder.interactions[-1].args[1]
)
def test_verbose_mode(self):
@@ -96,7 +103,7 @@ class TestMain(TestCase):
# result is something like "lpcraft, version 0.0.1"
self.assertIn(
- "lpcraft, version", emitter.recorder.interactions[0].args[1]
+ "lpcraft, version", emitter.recorder.interactions[-1].args[1]
)
def test_trace_mode(self):
@@ -111,5 +118,5 @@ class TestMain(TestCase):
# result is something like "lpcraft, version 0.0.1"
self.assertIn(
- "lpcraft, version", emitter.recorder.interactions[0].args[1]
+ "lpcraft, version", emitter.recorder.interactions[-1].args[1]
)
Follow ups
