From 36fa251d2b426b47b2ef87d61a1ddeca45f7bf73 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Tue, 19 Nov 2024 12:41:46 +0100 Subject: [PATCH 1/4] added legacy tests interface --- .../tests/unit/test_core_errors.py | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 services/dynamic-sidecar/tests/unit/test_core_errors.py diff --git a/services/dynamic-sidecar/tests/unit/test_core_errors.py b/services/dynamic-sidecar/tests/unit/test_core_errors.py new file mode 100644 index 00000000000..6e23d832f83 --- /dev/null +++ b/services/dynamic-sidecar/tests/unit/test_core_errors.py @@ -0,0 +1,35 @@ +# pylint:disable=broad-exception-caught +# pylint:disable=no-member + +from simcore_service_dynamic_sidecar.core.errors import ( + UnexpectedDockerError, + VolumeNotFoundError, +) +from starlette import status + + +def test_legacy_interface_unexpected_docker_error(): + message = "some_message" + status_code = 42 + try: + raise UnexpectedDockerError( # noqa: TRY301 + message=message, status_code=status_code + ) + except Exception as e: + print(e) + assert e.status_code == status_code # noqa: PT017 + assert message in e.message # noqa: PT017 + + +def test_legacy_interface_volume_not_found_error(): + try: + raise VolumeNotFoundError( # noqa: TRY301 + source_label="some", run_id="run_id", volumes=[{}, {"Name": "a_volume"}] + ) + except Exception as e: + print(e) + assert ( # noqa: PT017 + e.message + == "Expected 1 got 2 volumes labels with source_label='some', run_id='run_id': Found UNKNOWN a_volume" + ) + assert e.status_code == status.HTTP_404_NOT_FOUND # noqa: PT017 From b41b705be423649bc7da1f8240c02f78f3cb543f Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Tue, 19 Nov 2024 12:54:40 +0100 Subject: [PATCH 2/4] refactored errors --- .../core/docker_utils.py | 8 +++- .../core/error_handlers.py | 2 +- .../core/errors.py | 41 ++++--------------- .../core/settings.py | 13 +++--- .../tests/unit/test_core_errors.py | 11 ++++- 5 files changed, 34 insertions(+), 41 deletions(-) diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/docker_utils.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/docker_utils.py index 43959d5fba5..baa93fdb4cc 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/docker_utils.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/docker_utils.py @@ -50,7 +50,13 @@ async def get_volume_by_label(label: str, run_id: RunID) -> dict[str, Any]: volumes = data["Volumes"] _logger.debug("volumes query for label=%s volumes=%s", label, volumes) if len(volumes) != 1: - raise VolumeNotFoundError(label, run_id, volumes) + raise VolumeNotFoundError( + volume_count=len(volumes), + source_label=label, + run_id=run_id, + volume_names=" ".join(v.get("Name", "UNKNOWN") for v in volumes), + status_code=http_status.HTTP_404_NOT_FOUND, + ) volume_details: dict[str, Any] = volumes[0] return volume_details diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/error_handlers.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/error_handlers.py index 9fbceae96a0..cbd3e4dbe52 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/error_handlers.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/error_handlers.py @@ -12,7 +12,7 @@ async def http_error_handler( ) -> JSONResponse: return JSONResponse( content=jsonable_encoder({"errors": [exception.message]}), - status_code=exception.status_code, + status_code=exception.status_code, # type:ignore[attr-defined] ) diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/errors.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/errors.py index bd74b68dfb7..b9a449ecb36 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/errors.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/errors.py @@ -1,53 +1,30 @@ -from typing import Any - from common_library.errors_classes import OsparcErrorMixin -from fastapi import status -from models_library.services import RunID -class BaseDynamicSidecarError(Exception): +class BaseDynamicSidecarError(OsparcErrorMixin, Exception): """Used as base for all exceptions""" - def __init__( - self, nessage: str, status_code: int = status.HTTP_500_INTERNAL_SERVER_ERROR - ) -> None: - self.message: str = nessage - self.status_code: int = status_code - super().__init__(nessage) - class VolumeNotFoundError(BaseDynamicSidecarError): - def __init__( - self, source_label: str, run_id: RunID, volumes: list[dict[str, Any]] - ) -> None: - super().__init__( - f"Expected 1 got {len(volumes)} volumes labels with {source_label=}, {run_id=}: " - f"Found {' '.join(v.get('Name', 'UNKNOWN') for v in volumes)}", - status_code=status.HTTP_404_NOT_FOUND, - ) + msg_template = ( + "Expected 1 got {volume_count} volumes labels with " + "source_label={source_label}, run_id={run_id}: Found {volume_names}" + ) class UnexpectedDockerError(BaseDynamicSidecarError): - def __init__(self, message: str, status_code: int) -> None: - super().__init__( - f"An unexpected Docker error occurred {status_code=}, {message=}", - status_code=status_code, - ) - - -class BaseError(OsparcErrorMixin, BaseDynamicSidecarError): - ... + msg_template = "An unexpected Docker error occurred status_code={status_code}, message={message}" -class ContainerExecContainerNotFoundError(BaseError): +class ContainerExecContainerNotFoundError(BaseDynamicSidecarError): msg_template = "Container '{container_name}' was not found" -class ContainerExecTimeoutError(BaseError): +class ContainerExecTimeoutError(BaseDynamicSidecarError): msg_template = "Timed out after {timeout} while executing: '{command}'" -class ContainerExecCommandFailedError(BaseError): +class ContainerExecCommandFailedError(BaseDynamicSidecarError): msg_template = ( "Command '{command}' exited with code '{exit_code}'" "and output: '{command_result}'" diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py index af477d39ce4..1e54c8a4548 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py @@ -2,7 +2,7 @@ from datetime import timedelta from functools import lru_cache from pathlib import Path -from typing import cast +from typing import Annotated, cast from common_library.pydantic_validators import validate_numeric_string_as_timedelta from models_library.basic_types import BootModeEnum, PortInt @@ -53,10 +53,13 @@ class SystemMonitorSettings(BaseCustomSettings): class ApplicationSettings(BaseCustomSettings, MixinLoggingSettings): - SC_BOOT_MODE: BootModeEnum = Field( - ..., - description="boot mode helps determine if in development mode or normal operation", - ) + SC_BOOT_MODE: Annotated[ + BootModeEnum, + Field( + ..., + description="boot mode helps determine if in development mode or normal operation", + ), + ] DYNAMIC_SIDECAR_DY_VOLUMES_MOUNT_DIR: Path = Field( ..., diff --git a/services/dynamic-sidecar/tests/unit/test_core_errors.py b/services/dynamic-sidecar/tests/unit/test_core_errors.py index 6e23d832f83..7b112878c9c 100644 --- a/services/dynamic-sidecar/tests/unit/test_core_errors.py +++ b/services/dynamic-sidecar/tests/unit/test_core_errors.py @@ -23,13 +23,20 @@ def test_legacy_interface_unexpected_docker_error(): def test_legacy_interface_volume_not_found_error(): try: + volumes = [{}, {"Name": "a_volume"}] + volume_names = " ".join(v.get("Name", "UNKNOWN") for v in volumes) + raise VolumeNotFoundError( # noqa: TRY301 - source_label="some", run_id="run_id", volumes=[{}, {"Name": "a_volume"}] + volume_count=len(volumes), + source_label="some", + run_id="run_id", + volume_names=volume_names, + status_code=status.HTTP_404_NOT_FOUND, ) except Exception as e: print(e) assert ( # noqa: PT017 e.message - == "Expected 1 got 2 volumes labels with source_label='some', run_id='run_id': Found UNKNOWN a_volume" + == "Expected 1 got 2 volumes labels with source_label=some, run_id=run_id: Found UNKNOWN a_volume" ) assert e.status_code == status.HTTP_404_NOT_FOUND # noqa: PT017 From 03a37a60b0a18e05866f9c2b8142b8891f7b127b Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Tue, 19 Nov 2024 13:08:17 +0100 Subject: [PATCH 3/4] fixed unit test --- services/dynamic-sidecar/tests/unit/test_api_rest_containers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/dynamic-sidecar/tests/unit/test_api_rest_containers.py b/services/dynamic-sidecar/tests/unit/test_api_rest_containers.py index 719d2f38c6c..7ce7027c6e0 100644 --- a/services/dynamic-sidecar/tests/unit/test_api_rest_containers.py +++ b/services/dynamic-sidecar/tests/unit/test_api_rest_containers.py @@ -480,7 +480,7 @@ async def test_container_docker_error( def _expected_error_string(status_code: int) -> dict[str, Any]: return { "errors": [ - f"An unexpected Docker error occurred status_code={status_code}, message='aiodocker_mocked_error'" + f"An unexpected Docker error occurred status_code={status_code}, message=aiodocker_mocked_error" ] } From 24f8abd67dd02d354e04fe0b8985c40eabfbc458 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Tue, 19 Nov 2024 15:52:05 +0100 Subject: [PATCH 4/4] refactor settings --- .../core/settings.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py index 1e54c8a4548..795015e1520 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py @@ -2,10 +2,10 @@ from datetime import timedelta from functools import lru_cache from pathlib import Path -from typing import Annotated, cast +from typing import cast from common_library.pydantic_validators import validate_numeric_string_as_timedelta -from models_library.basic_types import BootModeEnum, PortInt +from models_library.basic_types import PortInt from models_library.callbacks_mapping import CallbacksMapping from models_library.products import ProductName from models_library.projects import ProjectID @@ -21,8 +21,8 @@ field_validator, ) from servicelib.logging_utils_filtering import LoggerName, MessageSubstring +from settings_library.application import BaseApplicationSettings from settings_library.aws_s3_cli import AwsS3CliSettings -from settings_library.base import BaseCustomSettings from settings_library.docker_registry import RegistrySettings from settings_library.node_ports import StorageAuthSettings from settings_library.postgres import PostgresSettings @@ -35,7 +35,7 @@ from settings_library.utils_logging import MixinLoggingSettings -class ResourceTrackingSettings(BaseCustomSettings): +class ResourceTrackingSettings(BaseApplicationSettings): RESOURCE_TRACKING_HEARTBEAT_INTERVAL: timedelta = Field( default=DEFAULT_RESOURCE_USAGE_HEARTBEAT_INTERVAL, description="each time the status of the service is propagated", @@ -46,20 +46,13 @@ class ResourceTrackingSettings(BaseCustomSettings): ) -class SystemMonitorSettings(BaseCustomSettings): +class SystemMonitorSettings(BaseApplicationSettings): DY_SIDECAR_SYSTEM_MONITOR_TELEMETRY_ENABLE: bool = Field( default=False, description="enabled/disabled disk usage monitoring" ) -class ApplicationSettings(BaseCustomSettings, MixinLoggingSettings): - SC_BOOT_MODE: Annotated[ - BootModeEnum, - Field( - ..., - description="boot mode helps determine if in development mode or normal operation", - ), - ] +class ApplicationSettings(BaseApplicationSettings, MixinLoggingSettings): DYNAMIC_SIDECAR_DY_VOLUMES_MOUNT_DIR: Path = Field( ...,