From e95641b11154812c4caa741cd3516618ea434dad Mon Sep 17 00:00:00 2001 From: Graham Hukill Date: Fri, 6 Sep 2024 13:52:00 -0400 Subject: [PATCH 1/2] Add CLI command init-job Why these changes are being introduced: The CLI is the primary interface for this application. We need a command that will initialize a new "job", where future CLI commands can then perform "runs" of. How this addresses that need: * Reworks CLI to for main group, callbacks, and debug ping command * Adds new init-job command * Updates README Side effects of this change: * CLI can create new jobs Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/TIMX-345 --- README.md | 57 ++++++++++++++++++++----- abdiff/cli.py | 106 +++++++++++++++++++++++++++++++++++++++++++--- tests/test_cli.py | 81 +++++++++++++++++++++++++++++++---- 3 files changed, 219 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index d5b9a32..9ca54c1 100644 --- a/README.md +++ b/README.md @@ -14,10 +14,11 @@ A **Job** in `abdiff` represents the A/B test for comparing the results from two ```json { - "job_name": "", - "transmogrifier_version_a": "", - "transmogrifier_version_b": "", - // any other data helpful to store about the job... + "job_directory": "amazing-job", + "job_message": "This job is testing all the things.", + "image_tag_a": "transmogrifier-example-job-1-abc123:latest", + "image_tag_b": "transmogrifier-example-job-1-def456:latest", + // potentially other job related data... } ``` @@ -27,15 +28,13 @@ A `run.json` follows roughly the following format, demonstrating fields added by ```json { - // all data from job.json included..., - "timestamp": "2024-08-23_15-55-00", - "transmogrifier_docker_image_a": "transmogrifier-job--version-a:latest", - "transmogrifier_docker_image_b": "transmogrifier-job--version-b:latest", + // all job data... + "timestamp": "2024-08-23_15-55-00", "input_files": [ "s3://path/to/extract_file_1.xml", "s3://path/to/extract_file_2.xml" ] - // any other data helpful to store about the run... + // potentially other run related data... } ``` @@ -63,7 +62,45 @@ The following sketches a single job `"test-refactor"` and two runs `"2024-08-23_ ## CLI commands -Coming soon... +### `abdiff` +```text +Usage: -c [OPTIONS] COMMAND [ARGS]... + +Options: + -v, --verbose Pass to log at debug level instead of info. + -h, --help Show this message and exit. + +Commands: + init-job Initialize a new Job. + ping Debug ping/pong command. +``` + +### `abdiff ping` +```text +Usage: -c ping [OPTIONS] + + Debug ping/pong command. + +Options: + -h, --help Show this message and exit. +``` + +### `abdiff init-job` +``` +Usage: -c init-job [OPTIONS] + + Initialize a new Job. + +Options: + -m, --message TEXT Message to describe Job. + -d, --job-directory TEXT Job working directory to create. [required] + -a, --commit-sha-a TEXT Transmogrifier commit SHA for version 'A' + [required] + -b, --commit-sha-b TEXT Transmogrifier commit SHA for version 'B' + [required] + -h, --help Show this message and exit. +``` + ## Development diff --git a/abdiff/cli.py b/abdiff/cli.py index 832d97c..e270bab 100644 --- a/abdiff/cli.py +++ b/abdiff/cli.py @@ -1,27 +1,119 @@ +import json import logging +from collections.abc import Callable from datetime import timedelta from time import perf_counter import click +from click.exceptions import ClickException from abdiff.config import configure_logger +from abdiff.core import build_ab_images +from abdiff.core import init_job as core_init_job +from abdiff.core.utils import read_job_json logger = logging.getLogger(__name__) -@click.command() +@click.group(context_settings={"help_option_names": ["-h", "--help"]}) @click.option( - "-v", "--verbose", is_flag=True, help="Pass to log at debug level instead of info" + "-v", + "--verbose", + is_flag=True, + help="Pass to log at debug level instead of info.", ) -def main(*, verbose: bool) -> None: - start_time = perf_counter() +@click.pass_context +def main( + ctx: click.Context, + verbose: bool, # noqa: FBT001 +) -> None: + ctx.ensure_object(dict) + ctx.obj["START_TIME"] = perf_counter() root_logger = logging.getLogger() logger.info(configure_logger(root_logger, verbose=verbose)) logger.info("Running process") - # Do things here! - elapsed_time = perf_counter() - start_time +@main.result_callback() +@click.pass_context +def post_main_group_subcommand( + ctx: click.Context, + *_args: tuple, + **_kwargs: dict, +) -> None: logger.info( - "Total time to complete process: %s", str(timedelta(seconds=elapsed_time)) + "Total elapsed: %s", + str( + timedelta(seconds=perf_counter() - ctx.obj["START_TIME"]), + ), ) + + +@main.command() +def ping() -> None: + """Debug ping/pong command.""" + logger.debug("got ping, preparing to pong") + click.echo("pong") + + +def shared_job_options(cli_command: Callable) -> Callable: + """Decorator to provide shared CLI arguments to Job related commands.""" + cli_command = click.option( + "-d", + "--job-directory", + type=str, + required=True, + help="Job working directory to create.", + )(cli_command) + + cli_command = click.option( + "-m", + "--message", + type=str, + required=False, + help="Message to describe Job.", + default="Not provided.", + )(cli_command) + + return cli_command # noqa: RET504 + + +@main.command() +@shared_job_options +@click.option( + "-a", + "--commit-sha-a", + type=str, + required=True, + help="Transmogrifier commit SHA for version 'A'", +) +@click.option( + "-b", + "--commit-sha-b", + type=str, + required=True, + help="Transmogrifier commit SHA for version 'B'", +) +def init_job( + job_directory: str, + commit_sha_a: str, + commit_sha_b: str, + message: str, +) -> None: + """Initialize a new Job.""" + try: + core_init_job(job_directory, message) + except FileExistsError as exc: + message = ( + f"Job directory already exists: '{job_directory}', cannot create new job." + ) + raise ClickException(message) from exc + + build_ab_images( + job_directory, + commit_sha_a, + commit_sha_b, + ) + + job_json = json.dumps(read_job_json(job_directory), indent=2) + logger.info(f"Job initialized: {job_json}") diff --git a/tests/test_cli.py b/tests/test_cli.py index 142da17..160a1bf 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,17 +1,82 @@ +import os +from unittest.mock import patch + from abdiff.cli import main +from abdiff.core.utils import read_job_json -def test_cli_no_options(caplog, runner): - result = runner.invoke(main) +def test_cli_default_log_level_info(caplog, runner): + result = runner.invoke(main, ["ping"]) assert result.exit_code == 0 assert "Logger 'root' configured with level=INFO" in caplog.text - assert "Running process" in caplog.text - assert "Total time to complete process" in caplog.text + assert "pong" in result.output -def test_cli_all_options(caplog, runner): - result = runner.invoke(main, ["--verbose"]) +def test_cli_verbose_sets_debug_log_level(caplog, runner): + caplog.set_level("DEBUG") + result = runner.invoke(main, ["--verbose", "ping"]) assert result.exit_code == 0 assert "Logger 'root' configured with level=DEBUG" in caplog.text - assert "Running process" in caplog.text - assert "Total time to complete process" in caplog.text + assert "got ping, preparing to pong" in caplog.text + + +def test_cli_main_group_callback_called(caplog, runner): + result = runner.invoke(main, ["--verbose", "ping"]) + assert result.exit_code == 0 + assert "Total elapsed" in caplog.text + + +@patch("abdiff.core.build_ab_images.docker_image_exists") +def test_init_job_all_arguments_success( + mocked_image_exists, + caplog, + runner, + job_directory, +): + mocked_image_exists.return_value = True + caplog.set_level("DEBUG") + + message = "This is a Super Job." + _result = runner.invoke( + main, + [ + "--verbose", + "init-job", + f"--job-directory={job_directory}", + f"--message={message}", + "--commit-sha-a=abc123", + "--commit-sha-b=def456", + ], + ) + + assert os.path.exists(job_directory) + + job_data = read_job_json(job_directory) + assert job_data == { + "job_directory": job_directory, + "job_message": message, + "image_tag_a": "transmogrifier-example-job-1-abc123:latest", + "image_tag_b": "transmogrifier-example-job-1-def456:latest", + } + + +def test_init_job_pre_existing_job_directory_raise_error( + caplog, + runner, + job_directory, +): + caplog.set_level("DEBUG") + os.makedirs(job_directory) + result = runner.invoke( + main, + [ + "--verbose", + "init-job", + f"--job-directory={job_directory}", + "--message=This is a Super Job.", + "--commit-sha-a=abc123", + "--commit-sha-b=def456", + ], + ) + assert result.exit_code == 1 + assert "Job directory already exists" in result.output From 14e8d8cf6ebb71e6b11a63a3772101017f13e36d Mon Sep 17 00:00:00 2001 From: Graham Hukill Date: Tue, 10 Sep 2024 09:24:15 -0400 Subject: [PATCH 2/2] Add docstring for CLI main group callback --- abdiff/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/abdiff/cli.py b/abdiff/cli.py index e270bab..8b63e8b 100644 --- a/abdiff/cli.py +++ b/abdiff/cli.py @@ -41,6 +41,7 @@ def post_main_group_subcommand( *_args: tuple, **_kwargs: dict, ) -> None: + """Callback for any work to perform after a main sub-command completes.""" logger.info( "Total elapsed: %s", str(