♻️ Propagate can_save when starting service (#4202)

packages/models-library/src/models_library/docker.py

@@ -1,26 +1,24 @@
 import re
-import warnings
-from typing import Any, Optional
 
 from models_library.generated_models.docker_rest_api import Task
 from models_library.products import ProductName
 from models_library.projects import ProjectID
 from models_library.projects_nodes import NodeID
 from models_library.users import UserID
-from pydantic import BaseModel, ConstrainedStr, Field, root_validator
+from pydantic import BaseModel, ConstrainedStr, Field
 
 from .basic_regex import DOCKER_GENERIC_TAG_KEY_RE, DOCKER_LABEL_KEY_REGEX
 
 
 class DockerLabelKey(ConstrainedStr):
     # NOTE: https://docs.docker.com/config/labels-custom-metadata/#key-format-recommendations
     # good practice: use reverse DNS notation
-    regex: Optional[re.Pattern[str]] = DOCKER_LABEL_KEY_REGEX
+    regex: re.Pattern[str] | None = DOCKER_LABEL_KEY_REGEX
 
 
 class DockerGenericTag(ConstrainedStr):
     # NOTE: https://docs.docker.com/engine/reference/commandline/tag/#description
-    regex: Optional[re.Pattern[str]] = DOCKER_GENERIC_TAG_KEY_RE
+    regex: re.Pattern[str] | None = DOCKER_GENERIC_TAG_KEY_RE
 
 
 class SimcoreServiceDockerLabelKeys(BaseModel):
@@ -32,27 +30,8 @@ class SimcoreServiceDockerLabelKeys(BaseModel):
     project_id: ProjectID = Field(..., alias="study_id")
     node_id: NodeID = Field(..., alias="uuid")
 
-    product_name: ProductName
-    simcore_user_agent: str
-
-    @root_validator(pre=True)
-    @classmethod
-    def ensure_defaults(cls, values: dict[str, Any]) -> dict[str, Any]:
-        warnings.warn(
-            (
-                "Once https://github.com/ITISFoundation/osparc-simcore/pull/3990 "
-                "reaches production this entire root_validator function "
-                "can be safely removed. Please check "
-                "https://github.com/ITISFoundation/osparc-simcore/issues/3996"
-            ),
-            DeprecationWarning,
-            stacklevel=2,
-        )
-        if values.get("product_name", None) is None:
-            values["product_name"] = "opsarc"
-        if values.get("simcore_user_agent", None) is None:
-            values["simcore_user_agent"] = ""
-        return values
+    product_name: ProductName = "opsarc"
+    simcore_user_agent: str = ""
 
     def to_docker_labels(self) -> dict[str, str]:
         """returns a dictionary of strings as required by docker"""

packages/models-library/src/models_library/products.py

@@ -1 +1,3 @@
-ProductName = str
+from typing import TypeAlias
+
+ProductName: TypeAlias = str

services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_services.py

@@ -143,6 +143,7 @@ async def create_dynamic_service(
             request_dns=x_dynamic_sidecar_request_dns,
             request_scheme=x_dynamic_sidecar_request_scheme,
             request_simcore_user_agent=x_simcore_user_agent,
+            can_save=service.can_save,
         )
 
     return await scheduler.get_stack_status(service.node_uuid)

services/director-v2/src/simcore_service_director_v2/models/domains/dynamic_services.py

@@ -37,6 +37,9 @@ class DynamicServiceCreate(ServiceDetails):
     service_resources: ServiceResourcesDict
 
     product_name: str = Field(..., description="Current product name")
+    can_save: bool = Field(
+        ..., description="the service data must be saved when closing"
+    )
 
     class Config:
         schema_extra = {
@@ -48,6 +51,7 @@ class Config:
                 "node_uuid": "75c7f3f4-18f9-4678-8610-54a2ade78eaa",
                 "basepath": "/x/75c7f3f4-18f9-4678-8610-54a2ade78eaa",
                 "product_name": "osparc",
+                "can_save": True,
                 "service_resources": ServiceResourcesDictHelpers.Config.schema_extra[
                     "examples"
                 ][0],

services/director-v2/src/simcore_service_director_v2/models/schemas/dynamic_services/scheduler.py

@@ -142,8 +142,8 @@ class ServiceRemovalState(BaseModel):
         False,
         description="when True, marks the service as ready to be removed",
     )
-    can_save: bool | None = Field(
-        None,
+    can_save: bool = Field(
+        ...,
         description="when True, saves the internal state and upload outputs of the service",
     )
     was_removed: bool = Field(
@@ -154,14 +154,32 @@ class ServiceRemovalState(BaseModel):
         ),
     )
 
-    def mark_to_remove(self, can_save: bool | None) -> None:
+    def mark_to_remove(self, can_save: bool) -> None:
         self.can_remove = True
         self.can_save = can_save
 
     def mark_removed(self) -> None:
         self.can_remove = False
         self.was_removed = True
 
+    @root_validator(pre=True)
+    @classmethod
+    def _can_save_is_no_longer_none(cls, values):
+        warnings.warn(
+            (
+                "Once https://github.com/ITISFoundation/osparc-simcore/issues/4202 "
+                "reaches production this entire root_validator function "
+                "can be safely removed. Please check "
+                "https://github.com/ITISFoundation/osparc-simcore/issues/4204"
+            ),
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        can_save: bool | None = values.get("can_save", None)
+        if can_save is None:
+            values["can_save"] = False
+        return values
+
 
 class DynamicSidecar(BaseModel):
     status: Status = Field(
@@ -411,26 +429,6 @@ def endpoint(self) -> AnyHttpUrl:
         "If set to None, the current product is undefined. Mostly for backwards compatibility",
     )
 
-    @root_validator(pre=True)
-    @classmethod
-    def _ensure_legacy_format_compatibility(cls, values):
-        warnings.warn(
-            (
-                "Once https://github.com/ITISFoundation/osparc-simcore/pull/3990 "
-                "reaches production this entire root_validator function "
-                "can be safely removed. Please check "
-                "https://github.com/ITISFoundation/osparc-simcore/issues/3996"
-            ),
-            DeprecationWarning,
-            stacklevel=2,
-        )
-        request_simcore_user_agent: str | None = values.get(
-            "request_simcore_user_agent"
-        )
-        if not request_simcore_user_agent:
-            values["request_simcore_user_agent"] = ""
-        return values
-
     @classmethod
     def from_http_request(
         # pylint: disable=too-many-arguments
@@ -441,6 +439,7 @@ def from_http_request(
         request_dns: str,
         request_scheme: str,
         request_simcore_user_agent: str,
+        can_save: bool,
         run_id: UUID | None = None,
     ) -> "SchedulerData":
         # This constructor method sets current product
@@ -467,7 +466,7 @@ def from_http_request(
             request_scheme=request_scheme,
             proxy_service_name=names_helper.proxy_service_name,
             request_simcore_user_agent=request_simcore_user_agent,
-            dynamic_sidecar={},
+            dynamic_sidecar={"service_removal_state": {"can_save": can_save}},
         )
         if run_id:
             obj_dict["run_id"] = run_id

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

@@ -79,6 +79,7 @@ async def add_service(
         request_dns: str,
         request_scheme: str,
         request_simcore_user_agent: str,
+        can_save: str,
     ) -> None:
         """
         Adds a new service.

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

@@ -153,6 +153,7 @@ async def add_service(
         request_dns: str,
         request_scheme: str,
         request_simcore_user_agent: str,
+        can_save: bool,
     ) -> None:
         """Invoked before the service is started"""
         scheduler_data = SchedulerData.from_http_request(
@@ -162,6 +163,7 @@ async def add_service(
             request_dns=request_dns,
             request_scheme=request_scheme,
             request_simcore_user_agent=request_simcore_user_agent,
+            can_save=can_save,
         )
         await self._add_service(scheduler_data)
 

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

@@ -70,6 +70,7 @@ async def add_service(
         request_dns: str,
         request_scheme: str,
         request_simcore_user_agent: str,
+        can_save: bool,
     ) -> None:
         return await self._scheduler.add_service(
             service,
@@ -78,6 +79,7 @@ async def add_service(
             request_dns,
             request_scheme,
             request_simcore_user_agent,
+            can_save,
         )
 
     def is_service_tracked(self, node_uuid: NodeID) -> bool:

services/director-v2/tests/integration/02/test_dynamic_services_routes.py

@@ -119,6 +119,7 @@ def start_request_data(
         service_version=dy_static_file_server_dynamic_sidecar_service["image"]["tag"],
         request_scheme="http",
         request_dns="localhost:50000",
+        can_save=True,
         settings=[
             {
                 "name": "resources",

services/director-v2/tests/integration/02/utils.py

@@ -5,7 +5,7 @@
 import logging
 import os
 import urllib.parse
-from typing import Any, Optional
+from typing import Any
 
 import aiodocker
 import httpx
@@ -121,7 +121,7 @@ async def _wait_for_service(service_name: str) -> None:
 
 
 async def _get_service_published_port(
-    service_name: str, target_port: Optional[int] = None
+    service_name: str, target_port: int | None = None
 ) -> int:
     # it takes a bit of time for the port to be auto generated
     # keep trying until it is there
@@ -215,7 +215,7 @@ async def patch_dynamic_service_url(app: FastAPI, node_uuid: str) -> str:
 
     # patch the endppoint inside the scheduler
     scheduler: DynamicSidecarsScheduler = app.state.dynamic_sidecar_scheduler
-    endpoint: Optional[str] = None
+    endpoint: str | None = None
     async with scheduler._scheduler._lock:  # pylint: disable=protected-access
         for (
             scheduler_data
@@ -267,7 +267,7 @@ async def assert_start_service(
     service_key: str,
     service_version: str,
     service_uuid: str,
-    basepath: Optional[str],
+    basepath: str | None,
     catalog_url: URL,
 ) -> None:
     service_resources: ServiceResourcesDict = await _get_service_resources(
@@ -281,6 +281,7 @@ async def assert_start_service(
         service_key=service_key,
         service_version=service_version,
         service_uuid=service_uuid,
+        can_save=True,
         basepath=basepath,
         service_resources=ServiceResourcesDictHelpers.create_jsonable(
             service_resources

services/director-v2/tests/mocks/legacy_scheduler_data_format.json

@@ -89,6 +89,7 @@
     },
     "request_dns": "master.com",
     "request_scheme": "https",
+    "request_simcore_user_agent": "",
     "proxy_service_name": "dy-proxy_12fb3055-db35-4a34-a9c0-bff1267aa859"
   },
   {
@@ -249,6 +250,7 @@
     },
     "request_dns": "master.com",
     "request_scheme": "https",
+    "request_simcore_user_agent": "",
     "proxy_service_name": "dy-proxy_d14bf3ea-abcf-52f2-8146-fc244b70f307"
   }
 ]

services/director-v2/tests/unit/conftest.py

@@ -95,6 +95,11 @@ def request_scheme() -> str:
     return "http"
 
 
+@pytest.fixture
+def can_save() -> bool:
+    return True
+
+
 @pytest.fixture
 def request_simcore_user_agent() -> str:
     return "python/test"
@@ -108,6 +113,7 @@ def scheduler_data_from_http_request(
     request_dns: str,
     request_scheme: str,
     request_simcore_user_agent: str,
+    can_save: bool,
     run_id: RunID,
 ) -> SchedulerData:
     return SchedulerData.from_http_request(
@@ -117,6 +123,7 @@ def scheduler_data_from_http_request(
         request_dns=request_dns,
         request_scheme=request_scheme,
         request_simcore_user_agent=request_simcore_user_agent,
+        can_save=can_save,
         run_id=run_id,
     )
 

services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py

@@ -83,14 +83,16 @@ async def observed_service(
     dynamic_sidecar_port: int,
     request_dns: str,
     request_scheme: str,
+    can_save: bool,
 ) -> SchedulerData:
     await dynamic_sidecar_scheduler.add_service(
         dynamic_service_create,
         simcore_service_labels,
         dynamic_sidecar_port,
         request_dns,
         request_scheme,
-        request_simcore_user_agent="",
+        "",
+        can_save,
     )
     # pylint:disable=protected-access
     return dynamic_sidecar_scheduler._scheduler.get_scheduler_data(

services/director-v2/tests/unit/with_dbs/test_modules_dynamic_sidecar_docker_service_specs.py

@@ -116,7 +116,7 @@ def expected_dynamic_sidecar_spec(
                         "is_service_environment_ready": False,
                         "service_removal_state": {
                             "can_remove": False,
-                            "can_save": None,
+                            "can_save": True,
                             "was_removed": False,
                         },
                         "status": {"current": "ok", "info": ""},

services/web/server/src/simcore_service_webserver/director_v2/_core_dynamic_services.py

@@ -83,6 +83,7 @@ async def run_dynamic_service(
     *,
     app: web.Application,
     product_name: str,
+    save_state: bool,
     user_id: PositiveInt,
     project_id: str,
     service_key: str,
@@ -100,6 +101,7 @@ async def run_dynamic_service(
     """
     data = {
         "product_name": product_name,
+        "can_save": save_state,
         "user_id": user_id,
         "project_id": project_id,
         "key": service_key,

services/web/server/src/simcore_service_webserver/projects/projects_api.py

@@ -220,6 +220,15 @@ async def _start_dynamic_service(
 
     # this is a dynamic node, let's gather its resources and start it
 
+    save_state = False
+    user_role: UserRole = await get_user_role(request.app, user_id)
+    if user_role > UserRole.GUEST:
+        save_state = await ProjectDBAPI.get_from_app_context(
+            request.app
+        ).has_permission(
+            user_id=user_id, project_uuid=f"{project_uuid}", permission="write"
+        )
+
     lock_key = _nodes_utils.get_service_start_lock_key(user_id, project_uuid)
     redis_client_sdk = get_redis_lock_manager_client_sdk(request.app)
     project_settings: ProjectsSettings = get_plugin_settings(request.app)
@@ -253,6 +262,7 @@ async def _start_dynamic_service(
         await director_v2_api.run_dynamic_service(
             app=request.app,
             product_name=product_name,
+            save_state=save_state,
             project_id=f"{project_uuid}",
             user_id=user_id,
             service_key=service_key,