♻️ Is638/dy sidecar refactoring (round 6) (ITISFoundation#3179)

Author: pcrespov

packages/service-library/src/servicelib/async_utils.py

@@ -1,10 +1,9 @@
 import asyncio
 import logging
 from collections import deque
+from dataclasses import dataclass
 from functools import wraps
-from typing import TYPE_CHECKING, Any, Callable, Deque, Dict, List, Optional
-
-import attr
+from typing import TYPE_CHECKING, Any, Callable, Deque, Optional
 
 logger = logging.getLogger(__name__)
 
@@ -22,15 +21,15 @@ class Queue(
         pass
 
 
-@attr.s(auto_attribs=True)
+@dataclass
 class Context:
     in_queue: asyncio.Queue
     out_queue: asyncio.Queue
     initialized: bool
     task: Optional[asyncio.Task] = None
 
 
-_sequential_jobs_contexts: Dict[str, Context] = {}
+_sequential_jobs_contexts: dict[str, Context] = {}
 
 
 async def stop_sequential_workers() -> None:
@@ -44,8 +43,8 @@ async def stop_sequential_workers() -> None:
 
 
 def run_sequentially_in_context(
-    target_args: List[str] = None,
-) -> Callable[[Any], Any]:
+    target_args: Optional[list[str]] = None,
+) -> Callable:
     """All request to function with same calling context will be run sequentially.
 
     Example:
@@ -83,10 +82,8 @@ async def func(param1, param2, param3):
     """
     target_args = [] if target_args is None else target_args
 
-    def internal(
-        decorated_function: Callable[[Any], Optional[Any]]
-    ) -> Callable[[Any], Optional[Any]]:
-        def get_context(args: Any, kwargs: Dict[Any, Any]) -> Context:
+    def decorator(decorated_function: Callable[[Any], Optional[Any]]):
+        def _get_context(args: Any, kwargs: dict) -> Context:
             arg_names = decorated_function.__code__.co_varnames[
                 : decorated_function.__code__.co_argcount
             ]
@@ -98,17 +95,17 @@ def get_context(args: Any, kwargs: Dict[Any, Any]) -> Context:
                 sub_args = arg.split(".")
                 main_arg = sub_args[0]
                 if main_arg not in search_args:
-                    message = (
+                    raise ValueError(
                         f"Expected '{main_arg}' in '{decorated_function.__name__}'"
                         f" arguments. Got '{search_args}'"
                     )
-                    raise ValueError(message)
                 context_key = search_args[main_arg]
                 for attribute in sub_args[1:]:
                     potential_key = getattr(context_key, attribute)
                     if not potential_key:
-                        message = f"Expected '{attribute}' attribute in '{context_key.__name__}' arguments."
-                        raise ValueError(message)
+                        raise ValueError(
+                            f"Expected '{attribute}' attribute in '{context_key.__name__}' arguments."
+                        )
                     context_key = potential_key
 
                 key_parts.append(f"{decorated_function.__name__}_{context_key}")
@@ -124,9 +121,10 @@ def get_context(args: Any, kwargs: Dict[Any, Any]) -> Context:
 
             return _sequential_jobs_contexts[key]
 
+        # --------------------
         @wraps(decorated_function)
         async def wrapper(*args: Any, **kwargs: Any) -> Any:
-            context: Context = get_context(args, kwargs)
+            context: Context = _get_context(args, kwargs)
 
             if not context.initialized:
                 context.initialized = True
@@ -164,4 +162,4 @@ async def worker(in_q: Queue, out_q: Queue) -> None:
 
         return wrapper
 
-    return internal
+    return decorator

packages/service-library/src/servicelib/fastapi/openapi.py

@@ -10,10 +10,13 @@
 
 from ..functools_utils import copy_func
 
-# Some common values for FastAPI(... server=[ ... ]) parameter
-# It will be added to the OpenAPI Specs (OAS).
+# SEE https://swagger.io/docs/specification/api-host-and-base-path/
+_OAS_DEFAULT_SERVER = {
+    "description": "Default server: requests directed to serving url",
+    "url": "/",
+}
 _OAS_DEVELOPMENT_SERVER = {
-    "description": "Development server",
+    "description": "Development server: can configure any base url",
     "url": "http://{host}:{port}",
     "variables": {
         "host": {"default": "127.0.0.1"},
@@ -24,14 +27,14 @@
 
 def get_common_oas_options(is_devel_mode: bool) -> dict[str, Any]:
     """common OAS options for FastAPI constructor"""
-    servers = None
+    servers = [
+        _OAS_DEFAULT_SERVER,
+    ]
     if is_devel_mode:
         # NOTE: for security, only exposed in devel mode
         # Make sure also that this is NOT used in edge services
         # SEE https://sonarcloud.io/project/security_hotspots?id=ITISFoundation_osparc-simcore&pullRequest=3165&hotspots=AYHPqDfX5LRQZ1Ko6y4-
-        servers = [
-            _OAS_DEVELOPMENT_SERVER,
-        ]
+        servers.append(_OAS_DEVELOPMENT_SERVER)
 
     return dict(
         servers=servers,

packages/service-library/tests/test_async_utils.py

@@ -1,15 +1,17 @@
 # pylint: disable=redefined-outer-name
 # pylint: disable=unused-argument
+# pylint: disable=unused-variable
 
 import asyncio
 import copy
 import random
 from collections import deque
 from dataclasses import dataclass
 from time import time
-from typing import Any, AsyncIterable, Dict, List, Optional
+from typing import Any, AsyncIterable, Optional
 
 import pytest
+from faker import Faker
 from servicelib.async_utils import (
     _sequential_jobs_contexts,
     run_sequentially_in_context,
@@ -30,8 +32,8 @@ async def ensure_run_in_sequence_context_is_empty() -> AsyncIterable[None]:
 
 
 @pytest.fixture
-def payload() -> str:
-    return "some string payload"
+def payload(faker: Faker) -> str:
+    return faker.text()
 
 
 @pytest.fixture
@@ -55,7 +57,7 @@ async def push(self, item: Any):
         async with self._lock:
             self._queue.append(item)
 
-    async def get_all(self) -> List[Any]:
+    async def get_all(self) -> list[Any]:
         async with self._lock:
             return list(self._queue)
 
@@ -78,7 +80,7 @@ async def orderly(c1: Any, c2: Any, c3: Any, control: Any) -> None:
         context = dict(c1=c1, c2=c2, c3=c3)
         await locked_stores[make_key_from_context(context)].push(control)
 
-    def make_key_from_context(context: Dict) -> str:
+    def make_key_from_context(context: dict) -> str:
         return ".".join([f"{k}:{v}" for k, v in context.items()])
 
     def make_context():

services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/task.py

@@ -365,8 +365,9 @@ async def observing_single_service(service_name: str) -> None:
 
                 error_code = create_error_code(e)
                 logger.exception(
-                    "Observation of %s unexpectedly failed",
-                    f"{service_name=}",
+                    "Observation of %s unexpectedly failed [%s]",
+                    f"{service_name=} ",
+                    f"{error_code}",
                     extra={"error_code": error_code},
                 )
                 scheduler_data.dynamic_sidecar.status.update_failing_status(

services/dynamic-sidecar/docker/boot.sh

@@ -6,11 +6,16 @@ IFS=$(printf '\n\t')
 
 INFO="INFO: [$(basename "$0")] "
 
-# BOOTING application ---------------------------------------------
 echo "$INFO" "Booting in ${SC_BOOT_MODE} mode ..."
 echo "$INFO" "User :$(id "$(whoami)")"
 echo "$INFO" "Workdir : $(pwd)"
 
+#
+# DEVELOPMENT MODE
+#
+# - prints environ info
+# - installs requirements in mounted volume
+#
 if [ "${SC_BUILD_TARGET}" = "development" ]; then
   echo "$INFO" "Environment :"
   printenv | sed 's/=/: /' | sed 's/^/    /' | sort
@@ -25,8 +30,10 @@ if [ "${SC_BUILD_TARGET}" = "development" ]; then
   pip list | sed 's/^/    /'
 fi
 
-# RUNNING application ----------------------------------------
-APP_LOG_LEVEL=${DIRECTOR_V2_LOGLEVEL:-${LOG_LEVEL:-${LOGLEVEL:-INFO}}}
+#
+# RUNNING application
+#
+APP_LOG_LEVEL=${DYNAMIC_SIDECAR_LOGLEVEL:-${LOG_LEVEL:-${LOGLEVEL:-INFO}}}
 SERVER_LOG_LEVEL=$(echo "${APP_LOG_LEVEL}" | tr '[:upper:]' '[:lower:]')
 echo "$INFO" "Log-level app/server: $APP_LOG_LEVEL/$SERVER_LOG_LEVEL"
 
@@ -46,5 +53,4 @@ else
   exec uvicorn simcore_service_dynamic_sidecar.main:the_app \
     --host 0.0.0.0 \
     --log-level "${SERVER_LOG_LEVEL}"
-
 fi

services/dynamic-sidecar/openapi.json

@@ -6,9 +6,13 @@
     "version": "1.1.0"
   },
   "servers": [
+    {
+      "url": "/",
+      "description": "Default server: requests directed to serving url"
+    },
     {
       "url": "http://{host}:{port}",
-      "description": "Development server",
+      "description": "Development server: can configure any base url",
       "variables": {
         "host": {
           "default": "127.0.0.1"

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/application.py

@@ -1,4 +1,5 @@
 import logging
+from typing import Optional
 
 from fastapi import FastAPI
 from models_library.basic_types import BootModeEnum
@@ -29,19 +30,68 @@
 # https://patorjk.com/software/taag/#p=display&f=AMC%20Tubes&t=DYSIDECAR
 #
 
-WELCOME_MSG = r"""
+APP_STARTED_BANNER_MSG = r"""
 d ss    Ss   sS   sss. d d ss    d sss     sSSs. d s.   d ss.
 S   ~o    S S   d      S S   ~o  S        S      S  ~O  S    b
 S     b    S    Y      S S     b S       S       S   `b S    P
 S     S    S      ss.  S S     S S sSSs  S       S sSSO S sS'
 S     P    S         b S S     P S       S       S    O S   S
 S    S     S         P S S    S  S        S      S    O S    S
-P ss"      P    ` ss'  P P ss"   P sSSss   "sss' P    P P    P   {}
+P ss"      P    ` ss'  P P ss"   P sSSss   "sss' P    P P    P   {} 🚀
 
 """.format(
     f"v{__version__}"
 )
 
+APP_FINISHED_BANNER_MSG = "{:=^100}".format("🎉 App shutdown completed 🎉")
+
+
+class AppState:
+    """Exposes states of an initialized app
+
+    Provides a stricter control on the read/write access
+    of the different app.state fields during the app's lifespan
+    """
+
+    _STATES = {
+        "settings": DynamicSidecarSettings,
+        "mounted_volumes": MountedVolumes,
+        "shared_store": SharedStore,
+    }
+
+    def __init__(self, initialized_app: FastAPI):
+        # guarantees app states are in place
+        errors = [
+            "app.state.{name}"
+            for name, type_ in AppState._STATES.items()
+            if not isinstance(getattr(initialized_app.state, name, None), type_)
+        ]
+        if errors:
+            raise ValueError(
+                f"These app states were not properly initialized: {errors}"
+            )
+
+        self._app = initialized_app
+
+    @property
+    def settings(self) -> DynamicSidecarSettings:
+        assert isinstance(self._app.state.settings, DynamicSidecarSettings)  # nosec
+        return self._app.state.settings
+
+    @property
+    def mounted_volumes(self) -> MountedVolumes:
+        assert isinstance(self._app.state.mounted_volumes, MountedVolumes)  # nosec
+        return self._app.state.mounted_volumes
+
+    @property
+    def shared_store(self) -> SharedStore:
+        assert isinstance(self._app.state.shared_store, SharedStore)  # nosec
+        return self._app.state.shared_store
+
+    @property
+    def compose_spec(self) -> Optional[str]:
+        return self.shared_store.compose_spec
+
 
 def setup_logger(settings: DynamicSidecarSettings):
     # SEE https://github.com/ITISFoundation/osparc-simcore/issues/3148
@@ -101,43 +151,33 @@ def create_app():
     app.add_exception_handler(BaseDynamicSidecarError, http_error_handler)
 
     # EVENTS ---------------------
+    app_state = AppState(app)
+
     async def _on_startup() -> None:
-        await login_registry(app.state.settings.REGISTRY_SETTINGS)
-        await volumes_fix_permissions(app.state.mounted_volumes)
-        print(WELCOME_MSG, flush=True)
+        await login_registry(app_state.settings.REGISTRY_SETTINGS)
+        await volumes_fix_permissions(app_state.mounted_volumes)
+        # STARTED
+        print(APP_STARTED_BANNER_MSG, flush=True)
 
     async def _on_shutdown() -> None:
-        logger.info("Going to remove spawned containers")
-        result = await docker_compose_down(
-            shared_store=app.state.shared_store,
-            settings=app.state.settings,
-            command_timeout=app.state.settings.DYNAMIC_SIDECAR_DOCKER_COMPOSE_DOWN_TIMEOUT,
-        )
-        logger.info("Container removal did_succeed=%s\n%s", result[0], result[1])
-
-        logger.info("shutdown cleanup completed")
+        if docker_compose_yaml := app_state.compose_spec:
+            logger.info("Removing spawned containers%s", docker_compose_yaml)
+
+            result = await docker_compose_down(
+                app.state.shared_store,
+                app.state.settings,
+                command_timeout=app.state.settings.DYNAMIC_SIDECAR_DOCKER_COMPOSE_DOWN_TIMEOUT,
+            )
+
+            logger.log(
+                logging.INFO if result.success else logging.ERROR,
+                "Removed spawned containers:\n%s",
+                result.decoded_stdout,
+            )
+        # FINISHED
+        print(APP_FINISHED_BANNER_MSG, flush=True)
 
     app.add_event_handler("startup", _on_startup)
     app.add_event_handler("shutdown", _on_shutdown)
 
     return app
-
-
-class AppState:
-    def __init__(self, app: FastAPI):
-        self._app = app
-
-    @property
-    def settings(self) -> DynamicSidecarSettings:
-        assert isinstance(self._app.state.settings, DynamicSidecarSettings)  # nosec
-        return self._app.state.settings
-
-    @property
-    def mounted_volumes(self) -> MountedVolumes:
-        assert isinstance(self._app.state.mounted_volumes, MountedVolumes)  # nosec
-        return self._app.state.mounted_volumes
-
-    @property
-    def shared_store(self) -> SharedStore:
-        assert isinstance(self._app.state.shared_store, SharedStore)  # nosec
-        return self._app.state.shared_store

services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/core/settings.py

@@ -32,7 +32,9 @@ class DynamicSidecarSettings(BaseCustomSettings, MixinLoggingSettings):
     )
 
     # LOGGING
-    LOG_LEVEL: str = Field(default="WARNING")
+    LOG_LEVEL: str = Field(
+        default="WARNING", env=["DYNAMIC_SIDECAR_LOGLEVEL", "LOG_LEVEL", "LOGLEVEL"]
+    )
 
     # SERVICE SERVER (see : https://www.uvicorn.org/settings/)
     DYNAMIC_SIDECAR_HOST: str = Field(