From 49e301c14cfaa6d1b5b95fec0555784767ddf4a0 Mon Sep 17 00:00:00 2001 From: Antoni Ivanov Date: Thu, 6 Apr 2023 13:52:45 +0300 Subject: [PATCH] vdk-control-cli: refactor output printing with printer class (#1819) Currently, if a command needs to print similar types of data in multiple places or formats, the developer would have to duplicate the printing code for each location or format. This would lead to a lot of redundant code, which is difficult to maintain and prone to errors. This is making the code more "DRY". Also, users and devs are limited to a fixed set of output formats provided by the application. This could be restricting if the devs needs to print data in a format that is not supported by the application. E.g I wanted to add `rich` or `streamlit` type potentially. By introducing support for customizable output formats with the Printer class and related functions, users can define their own output formats and register them with the application using the printer decorator. This allows users to print data in any format they desire, providing greater flexibility and customization options. This is of course an application of Factory and Strategy design patterns. This is adopted in a single command here only. In a separate PR, I will adopt this printer in the other commands. Testing Done: unit tests (incl new ones) Signed-off-by: Antoni Ivanov --- .gitignore | 1 + .../command_groups/job/deploy_cli_impl.py | 2 +- .../control/command_groups/job/execute.py | 2 +- .../control/command_groups/job/list.py | 2 +- .../control/command_groups/job/properties.py | 2 +- .../control/command_groups/job/show.py | 4 +- .../vdk/internal/control/utils/cli_utils.py | 35 +---- .../internal/control/utils/output_printer.py | 132 ++++++++++++++++++ .../control/utils/test_output_printer.py | 98 +++++++++++++ 9 files changed, 243 insertions(+), 35 deletions(-) create mode 100644 projects/vdk-control-cli/src/vdk/internal/control/utils/output_printer.py create mode 100644 projects/vdk-control-cli/tests/vdk/internal/control/utils/test_output_printer.py diff --git a/.gitignore b/.gitignore index 00021971bf..3004b9e10f 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ __pycache__/* .cache/* .*.swp */.ipynb_checkpoints/* +out .DS_Store .eggs build diff --git a/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/deploy_cli_impl.py b/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/deploy_cli_impl.py index 95fb36e709..08a90d838a 100644 --- a/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/deploy_cli_impl.py +++ b/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/deploy_cli_impl.py @@ -22,7 +22,7 @@ from vdk.internal.control.rest_lib.factory import ApiClientFactory from vdk.internal.control.rest_lib.rest_client_errors import ApiClientErrorDecorator from vdk.internal.control.utils.cli_utils import get_or_prompt -from vdk.internal.control.utils.cli_utils import OutputFormat +from vdk.internal.control.utils.output_printer import OutputFormat log = logging.getLogger(__name__) diff --git a/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/execute.py b/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/execute.py index 8d4b537ee6..1427be0827 100644 --- a/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/execute.py +++ b/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/execute.py @@ -22,7 +22,7 @@ from vdk.internal.control.rest_lib.rest_client_errors import ApiClientErrorDecorator from vdk.internal.control.utils import cli_utils from vdk.internal.control.utils.cli_utils import get_or_prompt -from vdk.internal.control.utils.cli_utils import OutputFormat +from vdk.internal.control.utils.output_printer import OutputFormat log = logging.getLogger(__name__) diff --git a/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/list.py b/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/list.py index 7029aaf094..75310fb8c6 100644 --- a/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/list.py +++ b/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/list.py @@ -16,7 +16,7 @@ from vdk.internal.control.rest_lib.rest_client_errors import ApiClientErrorDecorator from vdk.internal.control.utils import cli_utils from vdk.internal.control.utils.cli_utils import GqlQueryBuilder -from vdk.internal.control.utils.cli_utils import OutputFormat +from vdk.internal.control.utils.output_printer import OutputFormat log = logging.getLogger(__name__) diff --git a/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/properties.py b/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/properties.py index 0d19ddefae..713438e272 100644 --- a/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/properties.py +++ b/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/properties.py @@ -17,7 +17,7 @@ from vdk.internal.control.rest_lib.factory import ApiClientFactory from vdk.internal.control.rest_lib.rest_client_errors import ApiClientErrorDecorator from vdk.internal.control.utils import cli_utils -from vdk.internal.control.utils.cli_utils import OutputFormat +from vdk.internal.control.utils.output_printer import OutputFormat log = logging.getLogger(__name__) diff --git a/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/show.py b/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/show.py index 93e3c659bc..2b3623cf5f 100644 --- a/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/show.py +++ b/projects/vdk-control-cli/src/vdk/internal/control/command_groups/job/show.py @@ -14,7 +14,7 @@ from vdk.internal.control.rest_lib.factory import ApiClientFactory from vdk.internal.control.rest_lib.rest_client_errors import ApiClientErrorDecorator from vdk.internal.control.utils import cli_utils -from vdk.internal.control.utils.cli_utils import OutputFormat +from vdk.internal.control.utils.output_printer import json_format log = logging.getLogger(__name__) @@ -57,7 +57,7 @@ def show_job(self, job_name: str, team: str) -> None: job_as_dict["deployments"] = list(map(lambda d: d.to_dict(), deployments)) job_as_dict["executions"] = list(map(lambda e: e.to_dict(), executions))[:2] - click.echo(cli_utils.json_format(job_as_dict, indent=2)) + click.echo(json_format(job_as_dict, indent=2)) @click.command( diff --git a/projects/vdk-control-cli/src/vdk/internal/control/utils/cli_utils.py b/projects/vdk-control-cli/src/vdk/internal/control/utils/cli_utils.py index 764346d116..7ff044bf78 100644 --- a/projects/vdk-control-cli/src/vdk/internal/control/utils/cli_utils.py +++ b/projects/vdk-control-cli/src/vdk/internal/control/utils/cli_utils.py @@ -3,19 +3,16 @@ from __future__ import annotations import functools -import json import logging import os import shutil from dataclasses import dataclass -from enum import Enum -from enum import unique import click from vdk.internal.control.configuration.defaults_config import load_default_rest_api_url from vdk.internal.control.configuration.vdk_config import VDKConfig from vdk.internal.control.exception.vdk_exception import VDKException - +from vdk.internal.control.utils import output_printer log = logging.getLogger(__name__) @@ -96,16 +93,6 @@ def check(*args, **kwargs): return check -@unique -class OutputFormat(str, Enum): - """ - An enum used to specify the output formatting of a command. - """ - - TEXT = "TEXT" - JSON = "JSON" - - def output_option(*names, **kwargs): """ A decorator that adds an `--output, -o` option to the decorated command. @@ -118,8 +105,11 @@ def output_option(*names, **kwargs): def decorator(f): return click.option( *names, - type=click.Choice([e.value for e in OutputFormat], case_sensitive=False), - default=OutputFormat.TEXT.value, + type=click.Choice( + [e.upper() for e in output_printer._registered_printers.keys()], + case_sensitive=False, + ), + default="text", cls=extended_option(hide_if_default=True), help="The desirable format of the result. Supported formats include text and json.", **kwargs, @@ -128,19 +118,6 @@ def decorator(f): return decorator -def json_format(data, indent=None): - from datetime import date, datetime - - def json_serial(obj): - """JSON serializer for objects not serializable by default json code""" - - if isinstance(obj, (datetime, date)): - return obj.isoformat() - raise TypeError("Type %s not serializable" % type(obj)) - - return json.dumps(data, default=json_serial, indent=indent) - - def copy_directory(src, dst): if not os.path.exists(dst): os.makedirs(dst) diff --git a/projects/vdk-control-cli/src/vdk/internal/control/utils/output_printer.py b/projects/vdk-control-cli/src/vdk/internal/control/utils/output_printer.py new file mode 100644 index 0000000000..9959910af7 --- /dev/null +++ b/projects/vdk-control-cli/src/vdk/internal/control/utils/output_printer.py @@ -0,0 +1,132 @@ +# Copyright 2023-2023 VMware, Inc. +# SPDX-License-Identifier: Apache-2.0 +import abc +import json +from enum import Enum +from enum import unique +from typing import Any +from typing import Dict +from typing import List +from typing import Optional + +import click +from tabulate import tabulate + + +class Printer(abc.ABC): + """ + The abstract base class for all printer classes. + + A printer is responsible for printing data in a specific format, such as text or JSON. + Subclasses must implement the abstract methods below + """ + + @abc.abstractmethod + def print_table(self, data: Optional[List[Dict[str, Any]]]) -> None: + """ + Prints the table in the desired format (text, json, etc) + :param data: the table to print + """ + + @abc.abstractmethod + def print_dict(self, data: Optional[Dict[str, Any]]) -> None: + """ + Prints dictionary in the desired format (text, json, etc) + :param data: the dict to print + """ + + +""" +This dictionary contains all the registered printers. +The key is the output format and the value is the printer class. +Do not access this dictionary directly, use the printer decorator instead. +""" +_registered_printers = {} + + +def printer(output_format: str) -> callable: + """ + A decorator that registers a printer class for the given output format. + The class must implement the Printer interface and have a constructor with no parameters. + + :param output_format: The output format to register the printer for. + """ + + def decorator(cls): + _registered_printers[output_format.lower()] = cls + return cls + + return decorator + + +@printer("text") +class _PrinterText(Printer): + def print_table(self, table: Optional[List[Dict[str, Any]]]) -> None: + if table and len(table) > 0: + click.echo(tabulate(table, headers="keys", tablefmt="fancy_grid")) + else: + click.echo("No Data.") + + def print_dict(self, data: Optional[Dict[str, Any]]) -> None: + if data: + click.echo( + tabulate( + [[k, v] for k, v in data.items()], + headers=("key", "value"), + ) + ) + else: + click.echo("No Data.") + + +def json_format(data, indent=None): + from datetime import date, datetime + + def json_serial(obj): + """JSON serializer for objects not serializable by default json code""" + + if isinstance(obj, (datetime, date)): + return obj.isoformat() + raise TypeError("Type %s not serializable" % type(obj)) + + return json.dumps(data, default=json_serial, indent=indent) + + +@printer("json") +class _PrinterJson(Printer): + def print_table(self, data: List[Dict[str, Any]]) -> None: + if data: + click.echo(json_format(data)) + else: + click.echo("[]") + + def print_dict(self, data: Dict[str, Any]) -> None: + if data: + click.echo(json_format(data)) + else: + click.echo("{}") + + +def create_printer(output_format: str) -> Printer: + """ + Creates a printer instance for the given output format. + + :param output_format: the desired output format + :return: An instance of a Printer subclass that can print data in the desired format. + + """ + if output_format.lower() in _registered_printers: + printer_class = _registered_printers[output_format.lower()] + return printer_class() + else: + raise ValueError(f"Printer for output format {output_format} not registered") + + +@unique +class OutputFormat(str, Enum): + """ + An enum used to specify the output formatting of a command. + """ + + TEXT = "TEXT" + JSON = "JSON" diff --git a/projects/vdk-control-cli/tests/vdk/internal/control/utils/test_output_printer.py b/projects/vdk-control-cli/tests/vdk/internal/control/utils/test_output_printer.py new file mode 100644 index 0000000000..96a198299f --- /dev/null +++ b/projects/vdk-control-cli/tests/vdk/internal/control/utils/test_output_printer.py @@ -0,0 +1,98 @@ +# Copyright 2023-2023 VMware, Inc. +# SPDX-License-Identifier: Apache-2.0 +from typing import Any +from typing import Dict +from typing import List +from unittest.mock import patch + +import pytest +from vdk.internal.control.utils import output_printer +from vdk.internal.control.utils.output_printer import _PrinterJson +from vdk.internal.control.utils.output_printer import _PrinterText +from vdk.internal.control.utils.output_printer import create_printer +from vdk.internal.control.utils.output_printer import Printer + + +class TestPrinterText: + def test_print_dict(self): + with patch("click.echo") as mock_echo: + printer = _PrinterText() + data = {"key": "value"} + + printer.print_dict(data) + + expected_output = "key value\n" "----- -------\n" "key value" + mock_echo.assert_called_once_with(expected_output) + + def test_print_table_with_data(self): + with patch("click.echo") as mock_echo: + printer = _PrinterText() + + data = [{"key1": "value1", "key2": 2}, {"key1": "value3", "key2": 4}] + + printer.print_table(data) + + expected_output = ( + "╒════════╤════════╕\n" + "│ key1 │ key2 │\n" + "╞════════╪════════╡\n" + "│ value1 │ 2 │\n" + "├────────┼────────┤\n" + "│ value3 │ 4 │\n" + "╘════════╧════════╛" + ) + mock_echo.assert_called_once_with(expected_output) + + def test_print_table_with_no_data(self): + with patch("click.echo") as mock_echo: + printer = _PrinterText() + data = [] + + printer.print_table(data) + + expected_output = "No Data." + mock_echo.assert_called_once_with(expected_output) + + +class TestPrinterJson: + def test_print_dict(self): + with patch("click.echo") as mock_echo: + printer = _PrinterJson() + + data = {"key": "value"} + + printer.print_dict(data) + + expected_output = '{"key": "value"}' + mock_echo.assert_called_once_with(expected_output) + + def test_print_table(self): + with patch("click.echo") as mock_echo: + printer = _PrinterJson() + data = [ + {"key1": "value1", "key2": "value2"}, + {"key1": "value3", "key2": "value4"}, + ] + printer.print_table(data) + + expected_output = '[{"key1": "value1", "key2": "value2"}, {"key1": "value3", "key2": "value4"}]' + mock_echo.assert_called_once_with(expected_output) + + +class TestCreatePrinter: + def test_create_printer_with_registered_format(self): + class MockPrinter(Printer): + def print_dict(self, data: Dict[str, Any]) -> None: + pass + + def print_table(self, data: List[Dict[str, Any]]) -> None: + pass + + output_printer._registered_printers["mock"] = MockPrinter + + printer = create_printer("mock") + assert isinstance(printer, MockPrinter) + + def test_create_printer_with_unregistered_format(self): + with pytest.raises(ValueError): + create_printer("invalid_format")