From b53c5549cfb4cbd7584ba83e5fcfb91d66900b80 Mon Sep 17 00:00:00 2001 From: Paul Farault <paul@adaltas.com> Date: Tue, 3 Dec 2024 11:13:46 +0100 Subject: [PATCH 1/9] fix: custom message when browsing a non existing operation Part of #483 --- tdp/cli/commands/browse.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tdp/cli/commands/browse.py b/tdp/cli/commands/browse.py index ba69dafb..8abbddea 100644 --- a/tdp/cli/commands/browse.py +++ b/tdp/cli/commands/browse.py @@ -78,7 +78,16 @@ def browse( # Print a specific operation if deployment_id and operation: - _print_operations(dao.get_operation(deployment_id, operation)) + if operations := dao.get_operation(deployment_id, operation): + _print_operations(operations) + else: + click.echo( + f'Operation "{operation}" not found for deployment {deployment_id}' + ) + click.echo( + "Either the deployment does not exist or the operation is not" + + " found for the deployment." + ) return # Print a specific deployment From e2e0227246b2d38ecc27079e2e040bb2cc0367db Mon Sep 17 00:00:00 2001 From: Paul Farault <paul@adaltas.com> Date: Tue, 3 Dec 2024 11:14:23 +0100 Subject: [PATCH 2/9] refactor: rename Dao.get_operation to get_operations_by_name --- tdp/cli/commands/browse.py | 2 +- tdp/dao.py | 4 ++-- tests/unit/test_dao.py | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tdp/cli/commands/browse.py b/tdp/cli/commands/browse.py index 8abbddea..698cf7f1 100644 --- a/tdp/cli/commands/browse.py +++ b/tdp/cli/commands/browse.py @@ -78,7 +78,7 @@ def browse( # Print a specific operation if deployment_id and operation: - if operations := dao.get_operation(deployment_id, operation): + if operations := dao.get_operations_by_name(deployment_id, operation): _print_operations(operations) else: click.echo( diff --git a/tdp/dao.py b/tdp/dao.py index e5c2ebf1..f58f3dd5 100644 --- a/tdp/dao.py +++ b/tdp/dao.py @@ -240,10 +240,10 @@ def get_deployment(self, id: int) -> Optional[DeploymentModel]: self._check_session() return self.session.get(DeploymentModel, id) - def get_operation( + def get_operations_by_name( self, deployment_id: int, operation_name: str ) -> list[OperationModel]: - """Get an operation. + """Get all operations for a deployment from their name. Args: deployment_id: The deployment ID. diff --git a/tests/unit/test_dao.py b/tests/unit/test_dao.py index 681172b8..c94299b9 100644 --- a/tests/unit/test_dao.py +++ b/tests/unit/test_dao.py @@ -271,7 +271,9 @@ def test_operation(db_engine): session.commit() with Dao(db_engine) as dao: assert assert_equal_values_in_model( - dao.get_operation(deployment_id=1, operation_name="test_operation")[0], + dao.get_operations_by_name( + deployment_id=1, operation_name="test_operation" + )[0], OperationModel( deployment_id=1, operation_order=1, From 73f63f7ee9d5c0e895c6ab0d16d7f5f420627242 Mon Sep 17 00:00:00 2001 From: Paul Farault <paul@adaltas.com> Date: Tue, 3 Dec 2024 11:53:32 +0100 Subject: [PATCH 3/9] feat: retrieve an operation from its order in tdp browse command --- tdp/cli/commands/browse.py | 49 ++++++++++++++++++++++++++------------ tdp/dao.py | 15 ++++++++++++ 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/tdp/cli/commands/browse.py b/tdp/cli/commands/browse.py index 698cf7f1..f898bdf4 100644 --- a/tdp/cli/commands/browse.py +++ b/tdp/cli/commands/browse.py @@ -13,6 +13,7 @@ print_object, print_table, ) +from tdp.core.entities.operation import OperationName from tdp.core.models import DeploymentModel, OperationModel from tdp.dao import Dao @@ -78,16 +79,25 @@ def browse( # Print a specific operation if deployment_id and operation: - if operations := dao.get_operations_by_name(deployment_id, operation): - _print_operations(operations) - else: - click.echo( - f'Operation "{operation}" not found for deployment {deployment_id}' - ) - click.echo( - "Either the deployment does not exist or the operation is not" - + " found for the deployment." - ) + # Try to parse the operation argument as an integer + try: + operation_order = int(operation) + if record := dao.get_operation(deployment_id, operation_order): + _print_operation(record) + return + # If the operation argument is not an integer, consider that it is an + # operation name + except ValueError: + if operations := dao.get_operations_by_name(deployment_id, operation): + _print_operations(operations) + return + click.echo( + f'Operation "{operation}" not found for deployment {deployment_id}' + ) + click.echo( + "Either the deployment does not exist or the operation is not" + + " found for the deployment." + ) return # Print a specific deployment @@ -138,8 +148,17 @@ def _print_operations(operations: list[OperationModel]) -> None: # Print general operation infos click.secho("Operation(s) details", bold=True) for operation in operations: - click.echo(print_object(operation.to_dict())) - # Print operation records - if operation.logs: - click.secho("\nOperations", bold=True) - click.echo(str(operation.logs, "utf-8")) + _print_operation(operation) + click.echo("\n") + + +def _print_operation(operation: OperationModel) -> None: + """Print an operation in a human readable format. + + Args: + operation: Operation to print. + """ + print_object(operation.to_dict(filter_out=["logs"])) + if operation.logs: + click.secho("\nLogs", bold=True) + click.echo(str(operation.logs, "utf-8")) diff --git a/tdp/dao.py b/tdp/dao.py index f58f3dd5..8c5732d6 100644 --- a/tdp/dao.py +++ b/tdp/dao.py @@ -255,6 +255,21 @@ def get_operations_by_name( .all() ) + def get_operation( + self, deployment_id: int, operation_order: int + ) -> Optional[OperationModel]: + """Get an operation by deployment ID and operation order. + + Args: + deployment_id: The deployment ID. + operation_order: The operation order. + """ + return ( + self.session.query(OperationModel) + .filter_by(deployment_id=deployment_id, operation_order=operation_order) + .one_or_none() + ) + def get_planned_deployment(self) -> Optional[DeploymentModel]: self._check_session() return ( From 46fcc6d31f27b46616ceddbc3a0c98d3933dbe10 Mon Sep 17 00:00:00 2001 From: Paul Farault <paul@adaltas.com> Date: Tue, 3 Dec 2024 13:00:29 +0100 Subject: [PATCH 4/9] feat: try to parse operation name in tdp browse before looking for an operation --- tdp/cli/commands/browse.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tdp/cli/commands/browse.py b/tdp/cli/commands/browse.py index f898bdf4..c934c3e9 100644 --- a/tdp/cli/commands/browse.py +++ b/tdp/cli/commands/browse.py @@ -88,6 +88,11 @@ def browse( # If the operation argument is not an integer, consider that it is an # operation name except ValueError: + try: + OperationName.from_str(operation) + except ValueError: + click.echo(f"Operation {operation} is not a valid operation name.") + return if operations := dao.get_operations_by_name(deployment_id, operation): _print_operations(operations) return From 8ddc46965687031d314cf6e7561b2bb16fb81a0d Mon Sep 17 00:00:00 2001 From: Paul Farault <paul@adaltas.com> Date: Tue, 3 Dec 2024 13:40:42 +0100 Subject: [PATCH 5/9] feat: add message output when deployment not found on tdp browse --- tdp/cli/commands/browse.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tdp/cli/commands/browse.py b/tdp/cli/commands/browse.py index c934c3e9..9dfba054 100644 --- a/tdp/cli/commands/browse.py +++ b/tdp/cli/commands/browse.py @@ -110,6 +110,8 @@ def browse( deployment = dao.get_deployment(deployment_id) if deployment: _print_deployment(deployment) + else: + click.echo(f"Deployment {deployment_id} does not exist.") return # Print all deployments From 38d02eba0d881b6516f132db39c25997ed00a852 Mon Sep 17 00:00:00 2001 From: Paul Farault <paul@adaltas.com> Date: Tue, 3 Dec 2024 13:41:23 +0100 Subject: [PATCH 6/9] feat: print a specific message when multiple operation found for tdp browse --- tdp/cli/commands/browse.py | 30 +++++++++++++++--------------- tdp/cli/utils.py | 14 +++++++++++++- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/tdp/cli/commands/browse.py b/tdp/cli/commands/browse.py index 9dfba054..2091d224 100644 --- a/tdp/cli/commands/browse.py +++ b/tdp/cli/commands/browse.py @@ -11,6 +11,7 @@ from tdp.cli.utils import ( print_deployment, print_object, + print_operations, print_table, ) from tdp.core.entities.operation import OperationName @@ -93,8 +94,20 @@ def browse( except ValueError: click.echo(f"Operation {operation} is not a valid operation name.") return - if operations := dao.get_operations_by_name(deployment_id, operation): - _print_operations(operations) + operations = dao.get_operations_by_name(deployment_id, operation) + if len(operations) > 1: + click.secho( + f'Multiple operations "{operations[0].operation}" found for ' + + f"deployment {deployment_id}:", + bold=True, + ) + print_operations(operations, filter_out=["logs"]) + click.echo( + "\nUse the operation order to print a specific operation." + ) + return + elif len(operations) == 1: + _print_operation(operations[0]) return click.echo( f'Operation "{operation}" not found for deployment {deployment_id}' @@ -146,19 +159,6 @@ def _print_deployment(deployment: DeploymentModel) -> None: print_deployment(deployment, filter_out=["logs"]) -def _print_operations(operations: list[OperationModel]) -> None: - """Print a list of operations in a human readable format. - - Args: - operation: Operation to print. - """ - # Print general operation infos - click.secho("Operation(s) details", bold=True) - for operation in operations: - _print_operation(operation) - click.echo("\n") - - def _print_operation(operation: OperationModel) -> None: """Print an operation in a human readable format. diff --git a/tdp/cli/utils.py b/tdp/cli/utils.py index 51597098..e24dd8a2 100644 --- a/tdp/cli/utils.py +++ b/tdp/cli/utils.py @@ -11,6 +11,7 @@ from tabulate import tabulate from tdp.core.entities.hosted_entity_status import HostedEntityStatus +from tdp.core.models.operation_model import OperationModel if TYPE_CHECKING: from tdp.core.models import DeploymentModel @@ -51,8 +52,19 @@ def print_deployment( # Print deployment operations click.secho("\nOperations", bold=True) + print_operations(deployment.operations, filter_out=filter_out) + + +def print_operations( + operations: Iterable[OperationModel], /, *, filter_out: Optional[list[str]] = None +) -> None: + """Print a list of operations in a human readable format. + + Args: + operations: List of operations to print. + """ print_table( - [o.to_dict(filter_out=filter_out) for o in deployment.operations], + [o.to_dict(filter_out=filter_out) for o in operations], ) From 614736b714ee0b5e6dfdca0ed8ac6728013f5069 Mon Sep 17 00:00:00 2001 From: Paul Farault <paul@adaltas.com> Date: Tue, 3 Dec 2024 17:22:28 +0100 Subject: [PATCH 7/9] refactor: reorder tdp browse logic --- tdp/cli/commands/browse.py | 187 ++++++++++++++++++------------------- tdp/dao.py | 2 +- 2 files changed, 91 insertions(+), 98 deletions(-) diff --git a/tdp/cli/commands/browse.py b/tdp/cli/commands/browse.py index 2091d224..ad356eb3 100644 --- a/tdp/cli/commands/browse.py +++ b/tdp/cli/commands/browse.py @@ -1,7 +1,6 @@ # Copyright 2022 TOSIT.IO # SPDX-License-Identifier: Apache-2.0 -from collections.abc import Iterable from typing import Optional import click @@ -15,7 +14,6 @@ print_table, ) from tdp.core.entities.operation import OperationName -from tdp.core.models import DeploymentModel, OperationModel from tdp.dao import Dao @@ -60,112 +58,107 @@ def browse( ): """Browse deployments.""" with Dao(db_engine) as dao: - # Print last deployment plan + if plan and last: + raise click.BadOptionUsage( + "--plan", "Cannot use --plan and --last together." + ) + if deployment_id and (plan or last): + raise click.BadOptionUsage( + "deployment_id", "Cannot use deployment_id with --plan or --last." + ) + # Print the planned deployment if plan: - deployment_plan = dao.get_planned_deployment() - if deployment_plan: - _print_deployment(deployment_plan) - else: - click.echo("No planned deployment found") - click.echo("Create a deployment plan using the `tdp plan` command") + browse_plan(dao) return + # Print the last deployment elif last: - deployment = dao.get_last_deployment() - if deployment: - _print_deployment(deployment) - else: - click.echo("No deployment found") - click.echo("Create a deployment plan using the `tdp plan` command") + browse_last(dao) return - - # Print a specific operation - if deployment_id and operation: - # Try to parse the operation argument as an integer - try: - operation_order = int(operation) - if record := dao.get_operation(deployment_id, operation_order): - _print_operation(record) - return - # If the operation argument is not an integer, consider that it is an - # operation name - except ValueError: - try: - OperationName.from_str(operation) - except ValueError: - click.echo(f"Operation {operation} is not a valid operation name.") - return - operations = dao.get_operations_by_name(deployment_id, operation) - if len(operations) > 1: - click.secho( - f'Multiple operations "{operations[0].operation}" found for ' - + f"deployment {deployment_id}:", - bold=True, - ) - print_operations(operations, filter_out=["logs"]) - click.echo( - "\nUse the operation order to print a specific operation." - ) - return - elif len(operations) == 1: - _print_operation(operations[0]) - return - click.echo( - f'Operation "{operation}" not found for deployment {deployment_id}' - ) - click.echo( - "Either the deployment does not exist or the operation is not" - + " found for the deployment." - ) - return - # Print a specific deployment - if deployment_id: - deployment = dao.get_deployment(deployment_id) - if deployment: - _print_deployment(deployment) - else: - click.echo(f"Deployment {deployment_id} does not exist.") + elif deployment_id and not operation: + browse_deployment(dao, deployment_id) + return + # Print a specific operation + elif deployment_id and operation: + browse_operation(dao, deployment_id, operation) return - # Print all deployments - _print_deployments(dao.get_last_deployments(limit=limit, offset=offset)) + else: + browse_deployments(dao, limit, offset) -def _print_deployments(deployments: Iterable[DeploymentModel]) -> None: - """Print a list of deployments in a human readable format. - - Args: - deployments: List of deployments to print. - """ - if not deployments: - click.echo("No deployment found") +def browse_plan(dao: Dao) -> None: + if deployment_plan := dao.get_planned_deployment(): + print_deployment(deployment_plan, filter_out=["logs"]) + else: + click.echo("No planned deployment found") click.echo("Create a deployment plan using the `tdp plan` command") - print_table( - [d.to_dict(filter_out=["options"]) for d in deployments], - ) - - -def _print_deployment(deployment: DeploymentModel) -> None: - """Print a deployment in a human readable format. - - Args: - deployment: Deployment to print. - """ - if not deployment: - click.echo("Deployment does not exist.") - return - - print_deployment(deployment, filter_out=["logs"]) +def browse_last(dao: Dao) -> None: + if last_deployment := dao.get_last_deployment(): + print_deployment(last_deployment, filter_out=["logs"]) + else: + click.echo("No deployment found") + click.echo("Create a deployment plan using the `tdp plan` command") -def _print_operation(operation: OperationModel) -> None: - """Print an operation in a human readable format. - Args: - operation: Operation to print. - """ - print_object(operation.to_dict(filter_out=["logs"])) - if operation.logs: - click.secho("\nLogs", bold=True) - click.echo(str(operation.logs, "utf-8")) +def browse_deployment(dao: Dao, deployment_id: int) -> None: + if deployment := dao.get_deployment(deployment_id): + print_deployment(deployment, filter_out=["logs"]) + else: + click.echo(f"Deployment {deployment_id} does not exist.") + + +def browse_operation(dao: Dao, deployment_id: int, operation: str) -> None: + record = None + # Try to parse the operation argument as an integer + try: + operation_order = int(operation) + record = dao.get_operation(deployment_id, operation_order) + # If the operation argument is not an integer, consider that it is an + # operation name + except ValueError: + # Check if the operation name is valid + try: + OperationName.from_str(operation) + except ValueError as e: + raise click.BadParameter( + f"Operation {operation} is not a valid operation name." + ) from e + operations = dao.get_operations_by_name(deployment_id, operation) + if len(operations) == 1: + record = operations[0] + # If there are multiple operations with the given name, print them asking for a + # specific operation order + elif len(operations) > 1: + click.secho( + f'Multiple operations "{operations[0].operation}" found for ' + + f"deployment {deployment_id}:", + bold=True, + ) + print_operations(operations, filter_out=["logs"]) + click.echo("\nUse the operation order to print a specific operation.") + return + if record: + print_object(record.to_dict(filter_out=["logs"])) + if record.logs: + click.secho("\nLogs", bold=True) + click.echo(str(record.logs, "utf-8")) + else: + click.echo(f'Operation "{operation}" not found for deployment {deployment_id}') + click.echo( + "Either the deployment does not exist or the operation is not" + + " found for the deployment." + ) + + +def browse_deployments(dao: Dao, limit: int, offset: int) -> None: + deployments = dao.get_last_deployments(limit=limit, offset=offset) + if len(deployments) > 0: + print_table( + [d.to_dict(filter_out=["options"]) for d in deployments], + ) + else: + click.echo("No deployments found.") + click.echo("Create a deployment plan using the `tdp plan` command.") diff --git a/tdp/dao.py b/tdp/dao.py index 8c5732d6..a74c0bbf 100644 --- a/tdp/dao.py +++ b/tdp/dao.py @@ -287,7 +287,7 @@ def get_last_deployment(self) -> Optional[DeploymentModel]: def get_last_deployments( self, limit: Optional[int] = None, offset: Optional[int] = None - ) -> Iterable[DeploymentModel]: + ) -> list[DeploymentModel]: """Get last deployments in ascending order. Use limit and offset to paginate the results. From 12c95a37b7ec78893ae3db0c5c3456737d4d1da1 Mon Sep 17 00:00:00 2001 From: Paul Farault <paul@adaltas.com> Date: Tue, 3 Dec 2024 17:30:02 +0100 Subject: [PATCH 8/9] refactor: remove print_table and print_object utils --- tdp/cli/commands/browse.py | 36 ++++++++++++++++++++++++--------- tdp/cli/commands/status/show.py | 9 ++++++--- tdp/cli/utils.py | 35 +++++++++----------------------- 3 files changed, 43 insertions(+), 37 deletions(-) diff --git a/tdp/cli/commands/browse.py b/tdp/cli/commands/browse.py index ad356eb3..1dd34307 100644 --- a/tdp/cli/commands/browse.py +++ b/tdp/cli/commands/browse.py @@ -1,19 +1,21 @@ # Copyright 2022 TOSIT.IO # SPDX-License-Identifier: Apache-2.0 +from collections.abc import Iterable from typing import Optional import click from sqlalchemy import Engine +from tabulate import tabulate from tdp.cli.params import database_dsn_option from tdp.cli.utils import ( print_deployment, - print_object, print_operations, - print_table, ) from tdp.core.entities.operation import OperationName +from tdp.core.models.deployment_model import DeploymentModel +from tdp.core.models.operation_model import OperationModel from tdp.dao import Dao @@ -141,10 +143,7 @@ def browse_operation(dao: Dao, deployment_id: int, operation: str) -> None: click.echo("\nUse the operation order to print a specific operation.") return if record: - print_object(record.to_dict(filter_out=["logs"])) - if record.logs: - click.secho("\nLogs", bold=True) - click.echo(str(record.logs, "utf-8")) + _print_operation(record) else: click.echo(f'Operation "{operation}" not found for deployment {deployment_id}') click.echo( @@ -156,9 +155,28 @@ def browse_operation(dao: Dao, deployment_id: int, operation: str) -> None: def browse_deployments(dao: Dao, limit: int, offset: int) -> None: deployments = dao.get_last_deployments(limit=limit, offset=offset) if len(deployments) > 0: - print_table( - [d.to_dict(filter_out=["options"]) for d in deployments], - ) + _print_deployments(deployments) else: click.echo("No deployments found.") click.echo("Create a deployment plan using the `tdp plan` command.") + + +def _print_operation(operation: OperationModel) -> None: + click.echo( + tabulate( + operation.to_dict(filter_out=["logs"]).items(), + tablefmt="plain", + ) + ) + if operation.logs: + click.secho("\nLogs", bold=True) + click.echo(str(operation.logs, "utf-8")) + + +def _print_deployments(deployments: Iterable[DeploymentModel]) -> None: + click.echo( + tabulate( + [d.to_dict(filter_out=["options"]) for d in deployments], + headers="keys", + ) + ) diff --git a/tdp/cli/commands/status/show.py b/tdp/cli/commands/status/show.py index 8b03f4fc..031a6ccb 100644 --- a/tdp/cli/commands/status/show.py +++ b/tdp/cli/commands/status/show.py @@ -7,6 +7,7 @@ import click from sqlalchemy import Engine +from tabulate import tabulate from tdp.cli.params import ( collections_option, @@ -22,7 +23,6 @@ from tdp.cli.utils import ( check_services_cleanliness, print_hosted_entity_status_log, - print_table, ) from tdp.core.models.sch_status_log_model import SCHStatusLogModel from tdp.core.variables import ClusterVariables @@ -119,6 +119,9 @@ def show( def _print_sch_status_logs(sch_status: Iterable[SCHStatusLogModel]) -> None: - print_table( - [status.to_dict(filter_out=["id", "timestamp"]) for status in sch_status], + click.echo( + tabulate( + [status.to_dict(filter_out=["id", "timestamp"]) for status in sch_status], + headers="keys", + ) ) diff --git a/tdp/cli/utils.py b/tdp/cli/utils.py index e24dd8a2..a9078451 100644 --- a/tdp/cli/utils.py +++ b/tdp/cli/utils.py @@ -48,7 +48,12 @@ def print_deployment( ) -> None: # Print general deployment infos click.secho("Deployment details", bold=True) - click.echo(print_object(deployment.to_dict(filter_out=filter_out))) + click.echo( + tabulate( + deployment.to_dict(filter_out=filter_out).items(), + tablefmt="plain", + ) + ) # Print deployment operations click.secho("\nOperations", bold=True) @@ -63,43 +68,23 @@ def print_operations( Args: operations: List of operations to print. """ - print_table( - [o.to_dict(filter_out=filter_out) for o in operations], - ) - - -def print_object(obj: dict) -> None: - """Print an object in a human readable format. - - Args: - obj: Object to print. - """ click.echo( tabulate( - obj.items(), - tablefmt="plain", + [o.to_dict(filter_out=filter_out) for o in operations], + headers="keys", ) ) -def print_table(rows) -> None: - """Print a list of rows in a human readable format. - - Args: - rows: List of rows to print. - """ +def print_hosted_entity_status_log(sch_status: Iterable[HostedEntityStatus]) -> None: click.echo( tabulate( - rows, + [status.export_tabulate() for status in sch_status], headers="keys", ) ) -def print_hosted_entity_status_log(sch_status: Iterable[HostedEntityStatus]) -> None: - print_table([status.export_tabulate() for status in sch_status]) - - def _parse_line(line: str) -> tuple[str, Optional[str], Optional[list[str]]]: """Parses a line which contains an operation, and eventually a host and extra vars. From 65532d4440aae7a9e8d1589cc76e45cd986816e6 Mon Sep 17 00:00:00 2001 From: Paul Farault <paul@adaltas.com> Date: Tue, 3 Dec 2024 17:42:49 +0100 Subject: [PATCH 9/9] feat: do not display deployment_id in the operation table when printing a deployment --- tdp/cli/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tdp/cli/utils.py b/tdp/cli/utils.py index a9078451..b56b53ea 100644 --- a/tdp/cli/utils.py +++ b/tdp/cli/utils.py @@ -57,7 +57,9 @@ def print_deployment( # Print deployment operations click.secho("\nOperations", bold=True) - print_operations(deployment.operations, filter_out=filter_out) + print_operations( + deployment.operations, filter_out=[*(filter_out or []), "deployment_id"] + ) def print_operations(