From 52f3e660811ef3900495c0b4b86636f3361bc0bb Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Thu, 12 Jun 2025 13:28:18 +0200 Subject: [PATCH 01/33] moved start_task to TasksManager --- .../aiohttp/long_running_tasks/_server.py | 13 +- .../src/servicelib/long_running_tasks/task.py | 157 +++++++++--------- .../test_long_running_tasks.py | 5 +- ...test_long_running_tasks_context_manager.py | 5 +- .../test_long_running_tasks_task.py | 54 +++--- .../api/routes/dynamic_scheduler.py | 13 +- .../api/rest/containers_long_running_tasks.py | 28 +--- 7 files changed, 122 insertions(+), 153 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py index a68bb4a67b0..3f0a0be3afe 100644 --- a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py +++ b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py @@ -14,7 +14,7 @@ DEFAULT_STALE_TASK_DETECT_TIMEOUT, ) from ...long_running_tasks.models import TaskGet -from ...long_running_tasks.task import TaskContext, TaskProtocol, start_task +from ...long_running_tasks.task import TaskContext, TaskProtocol from ..typing_extension import Handler from . import _routes from ._constants import ( @@ -25,11 +25,11 @@ from ._manager import AiohttpLongRunningManager, get_long_running_manager -def no_ops_decorator(handler: Handler): +def _no_ops_decorator(handler: Handler): return handler -def no_task_context_decorator(handler: Handler): +def _no_task_context_decorator(handler: Handler): @wraps(handler) async def _wrap(request: web.Request): request[RQT_LONG_RUNNING_TASKS_CONTEXT_KEY] = {} @@ -55,8 +55,7 @@ async def start_long_running_task( task_name = _create_task_name_from_request(request_) task_id = None try: - task_id = start_task( - long_running_manager.tasks_manager, + task_id = long_running_manager.tasks_manager.start_task( task_, fire_and_forget=fire_and_forget, task_context=task_context, @@ -121,8 +120,8 @@ def setup( app: web.Application, *, router_prefix: str, - handler_check_decorator: Callable = no_ops_decorator, - task_request_context_decorator: Callable = no_task_context_decorator, + handler_check_decorator: Callable = _no_ops_decorator, + task_request_context_decorator: Callable = _no_task_context_decorator, stale_task_check_interval: datetime.timedelta = DEFAULT_STALE_TASK_CHECK_INTERVAL, stale_task_detect_timeout: datetime.timedelta = DEFAULT_STALE_TASK_DETECT_TIMEOUT, ) -> None: diff --git a/packages/service-library/src/servicelib/long_running_tasks/task.py b/packages/service-library/src/servicelib/long_running_tasks/task.py index c89e94ce476..539d1b6afe2 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/task.py +++ b/packages/service-library/src/servicelib/long_running_tasks/task.py @@ -34,6 +34,15 @@ TaskContext: TypeAlias = dict[str, Any] +class TaskProtocol(Protocol): + async def __call__( + self, progress: TaskProgress, *args: Any, **kwargs: Any + ) -> Any: ... + + @property + def __name__(self) -> str: ... + + async def _await_task(task: asyncio.Task) -> None: await task @@ -177,7 +186,7 @@ def list_tasks(self, with_task_context: TaskContext | None) -> list[TrackedTask] ) return tasks - def add_task( + def _add_task( self, task_name: TaskName, task: asyncio.Task, @@ -323,93 +332,85 @@ async def remove_task( finally: del self._tasks_groups[tracked_task.task_name][task_id] + def start_task( + self, + task: TaskProtocol, + *, + unique: bool = False, + task_context: TaskContext | None = None, + task_name: str | None = None, + fire_and_forget: bool = False, + **task_kwargs: Any, + ) -> TaskId: + """ + Creates a background task from an async function. -class TaskProtocol(Protocol): - async def __call__( - self, progress: TaskProgress, *args: Any, **kwargs: Any - ) -> Any: ... + An asyncio task will be created out of it by injecting a `TaskProgress` as the first + positional argument and adding all `handler_kwargs` as named parameters. - @property - def __name__(self) -> str: ... + NOTE: the progress is automatically bounded between 0 and 1 + NOTE: the `task` name must be unique in the module, otherwise when using + the unique parameter is True, it will not be able to distinguish between + them. + Args: + tasks_manager (TasksManager): the tasks manager + task (TaskProtocol): the tasks to be run in the background + unique (bool, optional): If True, then only one such named task may be run. Defaults to False. + task_context (Optional[TaskContext], optional): a task context storage can be retrieved during the task lifetime. Defaults to None. + task_name (Optional[str], optional): optional task name. Defaults to None. + fire_and_forget: if True, then the task will not be cancelled if the status is never called -def start_task( - tasks_manager: TasksManager, - task: TaskProtocol, - *, - unique: bool = False, - task_context: TaskContext | None = None, - task_name: str | None = None, - fire_and_forget: bool = False, - **task_kwargs: Any, -) -> TaskId: - """ - Creates a background task from an async function. + Raises: + TaskAlreadyRunningError: if unique is True, will raise if more than 1 such named task is started - An asyncio task will be created out of it by injecting a `TaskProgress` as the first - positional argument and adding all `handler_kwargs` as named parameters. + Returns: + TaskId: the task unique identifier + """ - NOTE: the progress is automatically bounded between 0 and 1 - NOTE: the `task` name must be unique in the module, otherwise when using - the unique parameter is True, it will not be able to distinguish between - them. + # NOTE: If not task name is given, it will be composed of the handler's module and it's name + # to keep the urls shorter and more meaningful. + handler_module = inspect.getmodule(task) + handler_module_name = handler_module.__name__ if handler_module else "" + task_name = task_name or f"{handler_module_name}.{task.__name__}" + task_name = urllib.parse.quote(task_name, safe="") + + # only one unique task can be running + if unique and self.is_task_running(task_name): + managed_tasks_ids = list(self.get_task_group(task_name).keys()) + assert len(managed_tasks_ids) == 1 # nosec + managed_task: TrackedTask = self.get_task_group(task_name)[ + managed_tasks_ids[0] + ] + raise TaskAlreadyRunningError( + task_name=task_name, managed_task=managed_task + ) - Args: - tasks_manager (TasksManager): the tasks manager - task (TaskProtocol): the tasks to be run in the background - unique (bool, optional): If True, then only one such named task may be run. Defaults to False. - task_context (Optional[TaskContext], optional): a task context storage can be retrieved during the task lifetime. Defaults to None. - task_name (Optional[str], optional): optional task name. Defaults to None. - fire_and_forget: if True, then the task will not be cancelled if the status is never called + task_id = self.create_task_id(task_name=task_name) + task_progress = TaskProgress.create(task_id=task_id) - Raises: - TaskAlreadyRunningError: if unique is True, will raise if more than 1 such named task is started + # bind the task with progress 0 and 1 + async def _progress_task(progress: TaskProgress, handler: TaskProtocol): + progress.update(message="starting", percent=0) + try: + return await handler(progress, **task_kwargs) + finally: + progress.update(message="finished", percent=1) - Returns: - TaskId: the task unique identifier - """ + async_task = asyncio.create_task( + _progress_task(task_progress, task), name=task_name + ) - # NOTE: If not task name is given, it will be composed of the handler's module and it's name - # to keep the urls shorter and more meaningful. - handler_module = inspect.getmodule(task) - handler_module_name = handler_module.__name__ if handler_module else "" - task_name = task_name or f"{handler_module_name}.{task.__name__}" - task_name = urllib.parse.quote(task_name, safe="") - - # only one unique task can be running - if unique and tasks_manager.is_task_running(task_name): - managed_tasks_ids = list(tasks_manager.get_task_group(task_name).keys()) - assert len(managed_tasks_ids) == 1 # nosec - managed_task: TrackedTask = tasks_manager.get_task_group(task_name)[ - managed_tasks_ids[0] - ] - raise TaskAlreadyRunningError(task_name=task_name, managed_task=managed_task) - - task_id = tasks_manager.create_task_id(task_name=task_name) - task_progress = TaskProgress.create(task_id=task_id) - - # bind the task with progress 0 and 1 - async def _progress_task(progress: TaskProgress, handler: TaskProtocol): - progress.update(message="starting", percent=0) - try: - return await handler(progress, **task_kwargs) - finally: - progress.update(message="finished", percent=1) - - async_task = asyncio.create_task( - _progress_task(task_progress, task), name=f"{task_name}" - ) - - tracked_task = tasks_manager.add_task( - task_name=task_name, - task=async_task, - task_progress=task_progress, - task_context=task_context or {}, - fire_and_forget=fire_and_forget, - task_id=task_id, - ) - - return tracked_task.task_id + tracked_task = self._add_task( + task_name=task_name, + task=async_task, + task_progress=task_progress, + task_context=task_context or {}, + fire_and_forget=fire_and_forget, + task_id=task_id, + ) + + return tracked_task.task_id __all__: tuple[str, ...] = ( diff --git a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py index 07809f928b0..027bac7b74a 100644 --- a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py +++ b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py @@ -31,7 +31,7 @@ TaskProgress, TaskStatus, ) -from servicelib.long_running_tasks.task import TaskContext, start_task +from servicelib.long_running_tasks.task import TaskContext from tenacity.asyncio import AsyncRetrying from tenacity.retry import retry_if_exception_type from tenacity.stop import stop_after_delay @@ -74,8 +74,7 @@ async def create_string_list_task( get_long_running_manager ), ) -> TaskId: - task_id = start_task( - long_running_manager.tasks_manager, + task_id = long_running_manager.tasks_manager.start_task( _string_list_task, num_strings=num_strings, sleep_time=sleep_time, diff --git a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py index 9072a70ddc0..02c2916f6e9 100644 --- a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py +++ b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py @@ -25,7 +25,6 @@ TaskId, TaskProgress, ) -from servicelib.long_running_tasks.task import start_task TASK_SLEEP_INTERVAL: Final[PositiveFloat] = 0.1 @@ -60,7 +59,7 @@ async def create_task_user_defined_route( FastAPILongRunningManager, Depends(get_long_running_manager) ], ) -> TaskId: - return start_task(long_running_manager.tasks_manager, task=a_test_task) + return long_running_manager.tasks_manager.start_task(task=a_test_task) @router.get("/api/failing", status_code=status.HTTP_200_OK) async def create_task_which_fails( @@ -68,7 +67,7 @@ async def create_task_which_fails( FastAPILongRunningManager, Depends(get_long_running_manager) ], ) -> TaskId: - return start_task(long_running_manager.tasks_manager, task=a_failing_test_task) + return long_running_manager.tasks_manager.start_task(task=a_failing_test_task) return router diff --git a/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py b/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py index 6e3ec1522e2..bf659087e20 100644 --- a/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py +++ b/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py @@ -23,7 +23,7 @@ TaskProgress, TaskStatus, ) -from servicelib.long_running_tasks.task import TasksManager, start_task +from servicelib.long_running_tasks.task import TasksManager from tenacity import TryAgain from tenacity.asyncio import AsyncRetrying from tenacity.retry import retry_if_exception_type @@ -83,8 +83,7 @@ async def tasks_manager() -> AsyncIterator[TasksManager]: async def test_task_is_auto_removed( tasks_manager: TasksManager, check_task_presence_before: bool ): - task_id = start_task( - tasks_manager, + task_id = tasks_manager.start_task( a_background_task, raise_when_finished=False, total_sleep=10 * TEST_CHECK_STALE_INTERVAL_S, @@ -111,8 +110,7 @@ async def test_task_is_auto_removed( async def test_checked_task_is_not_auto_removed(tasks_manager: TasksManager): - task_id = start_task( - tasks_manager, + task_id = tasks_manager.start_task( a_background_task, raise_when_finished=False, total_sleep=5 * TEST_CHECK_STALE_INTERVAL_S, @@ -126,8 +124,7 @@ async def test_checked_task_is_not_auto_removed(tasks_manager: TasksManager): async def test_fire_and_forget_task_is_not_auto_removed(tasks_manager: TasksManager): - task_id = start_task( - tasks_manager, + task_id = tasks_manager.start_task( a_background_task, raise_when_finished=False, total_sleep=5 * TEST_CHECK_STALE_INTERVAL_S, @@ -145,8 +142,7 @@ async def test_fire_and_forget_task_is_not_auto_removed(tasks_manager: TasksMana async def test_get_result_of_unfinished_task_raises(tasks_manager: TasksManager): - task_id = start_task( - tasks_manager, + task_id = tasks_manager.start_task( a_background_task, raise_when_finished=False, total_sleep=5 * TEST_CHECK_STALE_INTERVAL_S, @@ -159,11 +155,11 @@ async def test_unique_task_already_running(tasks_manager: TasksManager): async def unique_task(task_progress: TaskProgress): await asyncio.sleep(1) - start_task(tasks_manager=tasks_manager, task=unique_task, unique=True) + tasks_manager.start_task(task=unique_task, unique=True) # ensure unique running task regardless of how many times it gets started with pytest.raises(TaskAlreadyRunningError) as exec_info: - start_task(tasks_manager=tasks_manager, task=unique_task, unique=True) + tasks_manager.start_task(task=unique_task, unique=True) assert "must be unique, found: " in f"{exec_info.value}" @@ -172,7 +168,7 @@ async def not_unique_task(task_progress: TaskProgress): await asyncio.sleep(1) for _ in range(5): - start_task(tasks_manager=tasks_manager, task=not_unique_task) + tasks_manager.start_task(task=not_unique_task) def test_get_task_id(faker): @@ -182,8 +178,7 @@ def test_get_task_id(faker): async def test_get_status(tasks_manager: TasksManager): - task_id = start_task( - tasks_manager=tasks_manager, + task_id = tasks_manager.start_task( task=a_background_task, raise_when_finished=False, total_sleep=10, @@ -203,7 +198,7 @@ async def test_get_status_missing(tasks_manager: TasksManager): async def test_get_result(tasks_manager: TasksManager): - task_id = start_task(tasks_manager=tasks_manager, task=fast_background_task) + task_id = tasks_manager.start_task(task=fast_background_task) await asyncio.sleep(0.1) result = tasks_manager.get_task_result(task_id, with_task_context=None) assert result == 42 @@ -216,7 +211,7 @@ async def test_get_result_missing(tasks_manager: TasksManager): async def test_get_result_finished_with_error(tasks_manager: TasksManager): - task_id = start_task(tasks_manager=tasks_manager, task=failing_background_task) + task_id = tasks_manager.start_task(task=failing_background_task) # wait for result async for attempt in AsyncRetrying(**_RETRY_PARAMS): with attempt: @@ -229,8 +224,7 @@ async def test_get_result_finished_with_error(tasks_manager: TasksManager): async def test_get_result_task_was_cancelled_multiple_times( tasks_manager: TasksManager, ): - task_id = start_task( - tasks_manager=tasks_manager, + task_id = tasks_manager.start_task( task=a_background_task, raise_when_finished=False, total_sleep=10, @@ -245,8 +239,7 @@ async def test_get_result_task_was_cancelled_multiple_times( async def test_remove_task(tasks_manager: TasksManager): - task_id = start_task( - tasks_manager=tasks_manager, + task_id = tasks_manager.start_task( task=a_background_task, raise_when_finished=False, total_sleep=10, @@ -261,8 +254,7 @@ async def test_remove_task(tasks_manager: TasksManager): async def test_remove_task_with_task_context(tasks_manager: TasksManager): TASK_CONTEXT = {"some_context": "some_value"} - task_id = start_task( - tasks_manager=tasks_manager, + task_id = tasks_manager.start_task( task=a_background_task, raise_when_finished=False, total_sleep=10, @@ -294,8 +286,7 @@ async def test_remove_unknown_task(tasks_manager: TasksManager): async def test_cancel_task_with_task_context(tasks_manager: TasksManager): TASK_CONTEXT = {"some_context": "some_value"} - task_id = start_task( - tasks_manager=tasks_manager, + task_id = tasks_manager.start_task( task=a_background_task, raise_when_finished=False, total_sleep=10, @@ -321,8 +312,7 @@ async def test_list_tasks(tasks_manager: TasksManager): task_ids = [] for _ in range(NUM_TASKS): task_ids.append( # noqa: PERF401 - start_task( - tasks_manager=tasks_manager, + tasks_manager.start_task( task=a_background_task, raise_when_finished=False, total_sleep=10, @@ -337,21 +327,18 @@ async def test_list_tasks(tasks_manager: TasksManager): async def test_list_tasks_filtering(tasks_manager: TasksManager): - start_task( - tasks_manager=tasks_manager, + tasks_manager.start_task( task=a_background_task, raise_when_finished=False, total_sleep=10, ) - start_task( - tasks_manager=tasks_manager, + tasks_manager.start_task( task=a_background_task, raise_when_finished=False, total_sleep=10, task_context={"user_id": 213}, ) - start_task( - tasks_manager=tasks_manager, + tasks_manager.start_task( task=a_background_task, raise_when_finished=False, total_sleep=10, @@ -379,8 +366,7 @@ async def test_list_tasks_filtering(tasks_manager: TasksManager): async def test_define_task_name(tasks_manager: TasksManager, faker: Faker): task_name = faker.name() - task_id = start_task( - tasks_manager=tasks_manager, + task_id = tasks_manager.start_task( task=a_background_task, raise_when_finished=False, total_sleep=10, diff --git a/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py b/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py index 322ee491051..c947cf5bca6 100644 --- a/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py +++ b/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py @@ -13,7 +13,6 @@ TaskId, TaskProgress, ) -from servicelib.long_running_tasks.task import start_task from tenacity import retry from tenacity.before_sleep import before_sleep_log from tenacity.retry import retry_if_result @@ -112,8 +111,7 @@ async def _progress_callback( ) try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=_task_remove_service_containers, # type: ignore[arg-type] unique=True, node_uuid=node_uuid, @@ -173,8 +171,7 @@ async def _progress_callback( ) try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=_task_save_service_state, # type: ignore[arg-type] unique=True, node_uuid=node_uuid, @@ -216,8 +213,7 @@ async def _progress_callback( ) try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=_task_push_service_outputs, # type: ignore[arg-type] unique=True, node_uuid=node_uuid, @@ -254,8 +250,7 @@ async def _task_cleanup_service_docker_resources( ) try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=_task_cleanup_service_docker_resources, # type: ignore[arg-type] unique=True, node_uuid=node_uuid, diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py index 17ae8d9187f..0b97b6f5027 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py @@ -7,7 +7,6 @@ from servicelib.fastapi.requests_decorators import cancel_on_disconnect from servicelib.long_running_tasks.errors import TaskAlreadyRunningError from servicelib.long_running_tasks.models import TaskId -from servicelib.long_running_tasks.task import start_task from ...core.settings import ApplicationSettings from ...models.schemas.application_health import ApplicationHealth @@ -58,8 +57,7 @@ async def pull_user_servcices_docker_images( assert request # nosec try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=task_pull_user_servcices_docker_images, unique=True, app=app, @@ -99,8 +97,7 @@ async def create_service_containers_task( # pylint: disable=too-many-arguments assert request # nosec try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=task_create_service_containers, unique=True, settings=settings, @@ -133,8 +130,7 @@ async def runs_docker_compose_down_task( assert request # nosec try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=task_runs_docker_compose_down, unique=True, app=app, @@ -165,8 +161,7 @@ async def state_restore_task( assert request # nosec try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=task_restore_state, unique=True, settings=settings, @@ -196,8 +191,7 @@ async def state_save_task( assert request # nosec try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=task_save_state, unique=True, settings=settings, @@ -229,8 +223,7 @@ async def ports_inputs_pull_task( assert request # nosec try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=task_ports_inputs_pull, unique=True, port_keys=port_keys, @@ -262,8 +255,7 @@ async def ports_outputs_pull_task( assert request # nosec try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=task_ports_outputs_pull, unique=True, port_keys=port_keys, @@ -292,8 +284,7 @@ async def ports_outputs_push_task( assert request # nosec try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=task_ports_outputs_push, unique=True, outputs_manager=outputs_manager, @@ -322,8 +313,7 @@ async def containers_restart_task( assert request # nosec try: - return start_task( - long_running_manager.tasks_manager, + return long_running_manager.tasks_manager.start_task( task=task_containers_restart, unique=True, app=app, From 9df9acdba2a687867b505df26785fafcfba32b4b Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Thu, 12 Jun 2025 16:26:41 +0200 Subject: [PATCH 02/33] refactored to remove job_name and merge unique in task_id --- .../api_schemas_long_running_tasks/tasks.py | 9 +- .../aiohttp/long_running_tasks/_routes.py | 1 - .../fastapi/long_running_tasks/_routes.py | 1 - .../http_endpoint_responses.py | 21 ++- .../servicelib/long_running_tasks/models.py | 3 - .../src/servicelib/long_running_tasks/task.py | 131 ++++++++---------- .../test_long_running_tasks.py | 5 - .../test_long_running_tasks_task.py | 18 +-- .../simcore_service_webserver/tasks/_rest.py | 2 - .../02/test_projects_cancellations.py | 2 - 10 files changed, 85 insertions(+), 108 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py index 392d8841451..aeaa710d4c4 100644 --- a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py +++ b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py @@ -1,8 +1,7 @@ -import urllib.parse from datetime import datetime from typing import Any -from pydantic import BaseModel, field_validator +from pydantic import BaseModel from .base import TaskId, TaskProgress @@ -20,15 +19,9 @@ class TaskResult(BaseModel): class TaskBase(BaseModel): task_id: TaskId - task_name: str class TaskGet(TaskBase): status_href: str result_href: str abort_href: str - - @field_validator("task_name") - @classmethod - def unquote_str(cls, v) -> str: - return urllib.parse.unquote(v) diff --git a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_routes.py b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_routes.py index 513203f6a1e..11f7dcd4d7e 100644 --- a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_routes.py +++ b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_routes.py @@ -24,7 +24,6 @@ async def list_tasks(request: web.Request) -> web.Response: [ TaskGet( task_id=t.task_id, - task_name=t.task_name, status_href=f"{request.app.router['get_task_status'].url_for(task_id=t.task_id)}", result_href=f"{request.app.router['get_task_result'].url_for(task_id=t.task_id)}", abort_href=f"{request.app.router['cancel_and_delete_task'].url_for(task_id=t.task_id)}", diff --git a/packages/service-library/src/servicelib/fastapi/long_running_tasks/_routes.py b/packages/service-library/src/servicelib/fastapi/long_running_tasks/_routes.py index 8b474c8add9..ff82a9ca956 100644 --- a/packages/service-library/src/servicelib/fastapi/long_running_tasks/_routes.py +++ b/packages/service-library/src/servicelib/fastapi/long_running_tasks/_routes.py @@ -23,7 +23,6 @@ async def list_tasks( return [ TaskGet( task_id=t.task_id, - task_name=t.task_name, status_href=str(request.url_for("get_task_status", task_id=t.task_id)), result_href=str(request.url_for("get_task_result", task_id=t.task_id)), abort_href=str( diff --git a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py b/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py index be873f1a1a2..056ca5a3bab 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py +++ b/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py @@ -10,7 +10,7 @@ def list_tasks( tracked_tasks: list[TrackedTask] = tasks_manager.list_tasks( with_task_context=task_context ) - return [TaskBase(task_id=t.task_id, task_name=t.task_name) for t in tracked_tasks] + return [TaskBase(task_id=t.task_id) for t in tracked_tasks] def get_task_status( @@ -39,3 +39,22 @@ async def remove_task( tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId ) -> None: await tasks_manager.remove_task(task_id, with_task_context=task_context) + + +# TODO reroute start via this with registration of hanlders + +# TODO: to support this we need handler registration capability in order to figure out if they can be started +# this should be done via the final TasksManager +# - also + +# TODO: this should be used to start everywhere not the client's start_task actually, +# this way we completly isolate everything and are allowed to call it form everywhere +# async def start_task( +# tasks_manager: TasksManager, +# task: TaskContext | None, +# task_context: TaskContext | None = None, +# **task_kwargs: Any, +# ) -> TaskId: +# return tasks_manager.start_task( +# task=task, task_context=task_context, **task_kwargs +# ) diff --git a/packages/service-library/src/servicelib/long_running_tasks/models.py b/packages/service-library/src/servicelib/long_running_tasks/models.py index 37fc968568d..15ab97515af 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/models.py +++ b/packages/service-library/src/servicelib/long_running_tasks/models.py @@ -19,8 +19,6 @@ ) from pydantic import BaseModel, ConfigDict, Field, PositiveFloat -TaskName: TypeAlias = str - TaskType: TypeAlias = Callable[..., Coroutine[Any, Any, Any]] ProgressCallback: TypeAlias = Callable[ @@ -33,7 +31,6 @@ class TrackedTask(BaseModel): task_id: str task: Task - task_name: TaskName task_progress: TaskProgress # NOTE: this context lifetime is with the tracked task (similar to aiohttp storage concept) task_context: dict[str, Any] diff --git a/packages/service-library/src/servicelib/long_running_tasks/task.py b/packages/service-library/src/servicelib/long_running_tasks/task.py index 539d1b6afe2..77ba5ba9456 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/task.py +++ b/packages/service-library/src/servicelib/long_running_tasks/task.py @@ -22,14 +22,20 @@ TaskNotCompletedError, TaskNotFoundError, ) -from .models import TaskId, TaskName, TaskStatus, TrackedTask +from .models import TaskId, TaskStatus, TrackedTask _logger = logging.getLogger(__name__) + +_DEFAULT_NAMESPACE: Final = ( + "lrt" # NOTE: for now only one is used, will change in future PRs +) + _CANCEL_TASK_TIMEOUT: Final[PositiveFloat] = datetime.timedelta( seconds=1 ).total_seconds() +Namespace: TypeAlias = str TrackedTaskGroupDict: TypeAlias = dict[TaskId, TrackedTask] TaskContext: TypeAlias = dict[str, Any] @@ -48,29 +54,27 @@ async def _await_task(task: asyncio.Task) -> None: def _get_tasks_to_remove( - tasks_groups: dict[TaskName, TrackedTaskGroupDict], + tracked_tasks: TrackedTaskGroupDict, stale_task_detect_timeout_s: PositiveFloat, ) -> list[TaskId]: utc_now = datetime.datetime.now(tz=datetime.UTC) tasks_to_remove: list[TaskId] = [] - for tasks in tasks_groups.values(): - for task_id, tracked_task in tasks.items(): - if tracked_task.fire_and_forget: - continue - - if tracked_task.last_status_check is None: - # the task just added or never received a poll request - elapsed_from_start = (utc_now - tracked_task.started).seconds - if elapsed_from_start > stale_task_detect_timeout_s: - tasks_to_remove.append(task_id) - else: - # the task status was already queried by the client - elapsed_from_last_poll = ( - utc_now - tracked_task.last_status_check - ).seconds - if elapsed_from_last_poll > stale_task_detect_timeout_s: - tasks_to_remove.append(task_id) + + for task_id, tracked_task in tracked_tasks.items(): + if tracked_task.fire_and_forget: + continue + + if tracked_task.last_status_check is None: + # the task just added or never received a poll request + elapsed_from_start = (utc_now - tracked_task.started).seconds + if elapsed_from_start > stale_task_detect_timeout_s: + tasks_to_remove.append(task_id) + else: + # the task status was already queried by the client + elapsed_from_last_poll = (utc_now - tracked_task.last_status_check).seconds + if elapsed_from_last_poll > stale_task_detect_timeout_s: + tasks_to_remove.append(task_id) return tasks_to_remove @@ -83,9 +87,11 @@ def __init__( self, stale_task_check_interval: datetime.timedelta, stale_task_detect_timeout: datetime.timedelta, + namespace: Namespace = _DEFAULT_NAMESPACE, ): + self.namespace = namespace # Task groups: Every taskname maps to multiple asyncio.Task within TrackedTask model - self._tasks_groups: dict[TaskName, TrackedTaskGroupDict] = {} + self._tracked_tasks: TrackedTaskGroupDict = {} self.stale_task_check_interval = stale_task_check_interval self.stale_task_detect_timeout_s: PositiveFloat = ( @@ -104,9 +110,8 @@ async def setup(self) -> None: async def teardown(self) -> None: task_ids_to_remove: deque[TaskId] = deque() - for tasks_dict in self._tasks_groups.values(): - for tracked_task in tasks_dict.values(): - task_ids_to_remove.append(tracked_task.task_id) + for tracked_task in self._tracked_tasks.values(): + task_ids_to_remove.append(tracked_task.task_id) for task_id in task_ids_to_remove: # when closing we do not care about pending errors @@ -118,9 +123,6 @@ async def teardown(self) -> None: self._stale_tasks_monitor_task, max_delay=_CANCEL_TASK_TIMEOUT ) - def get_task_group(self, task_name: TaskName) -> TrackedTaskGroupDict: - return self._tasks_groups[task_name] - async def _stale_tasks_monitor_worker(self) -> None: """ A task is considered stale, if the task status is not queried @@ -139,7 +141,7 @@ async def _stale_tasks_monitor_worker(self) -> None: # will not be the case. tasks_to_remove = _get_tasks_to_remove( - self._tasks_groups, self.stale_task_detect_timeout_s + self._tracked_tasks, self.stale_task_detect_timeout_s ) # finally remove tasks and warn @@ -158,37 +160,18 @@ async def _stale_tasks_monitor_worker(self) -> None: task_id, with_task_context=None, reraise_errors=False ) - @staticmethod - def create_task_id(task_name: TaskName) -> str: - assert len(task_name) > 0 - return f"{task_name}.{uuid4()}" - - def is_task_running(self, task_name: TaskName) -> bool: - """returns True if a task named `task_name` is running""" - if task_name not in self._tasks_groups: - return False - - managed_tasks_ids = list(self._tasks_groups[task_name].keys()) - return len(managed_tasks_ids) > 0 - def list_tasks(self, with_task_context: TaskContext | None) -> list[TrackedTask]: - tasks: list[TrackedTask] = [] - for task_group in self._tasks_groups.values(): - if not with_task_context: - tasks.extend(task_group.values()) - else: - tasks.extend( - [ - task - for task in task_group.values() - if task.task_context == with_task_context - ] - ) - return tasks + if not with_task_context: + return list(self._tracked_tasks.values()) + + return [ + task + for task in self._tracked_tasks.values() + if task.task_context == with_task_context + ] def _add_task( self, - task_name: TaskName, task: asyncio.Task, task_progress: TaskProgress, task_context: TaskContext, @@ -196,33 +179,30 @@ def _add_task( *, fire_and_forget: bool, ) -> TrackedTask: - if task_name not in self._tasks_groups: - self._tasks_groups[task_name] = {} tracked_task = TrackedTask( task_id=task_id, task=task, - task_name=task_name, task_progress=task_progress, task_context=task_context, fire_and_forget=fire_and_forget, ) - self._tasks_groups[task_name][task_id] = tracked_task + self._tracked_tasks[task_id] = tracked_task return tracked_task def _get_tracked_task( self, task_id: TaskId, with_task_context: TaskContext | None ) -> TrackedTask: - for tasks in self._tasks_groups.values(): - if task_id in tasks: - if with_task_context and ( - tasks[task_id].task_context != with_task_context - ): - raise TaskNotFoundError(task_id=task_id) - return tasks[task_id] + if task_id not in self._tracked_tasks: + raise TaskNotFoundError(task_id=task_id) + + task = self._tracked_tasks[task_id] - raise TaskNotFoundError(task_id=task_id) + if with_task_context and task.task_context != with_task_context: + raise TaskNotFoundError(task_id=task_id) + + return task def get_task_status( self, task_id: TaskId, with_task_context: TaskContext | None @@ -330,7 +310,11 @@ async def remove_task( tracked_task.task, task_id, reraise_errors=reraise_errors ) finally: - del self._tasks_groups[tracked_task.task_name][task_id] + del self._tracked_tasks[task_id] + + def _get_task_id(self, task_name: str, *, is_unique: bool) -> TaskId: + unique_part = "unique" if is_unique else f"{uuid4()}" + return f"{self.namespace}_{task_name}_{unique_part}" def start_task( self, @@ -375,18 +359,14 @@ def start_task( task_name = task_name or f"{handler_module_name}.{task.__name__}" task_name = urllib.parse.quote(task_name, safe="") + task_id = self._get_task_id(task_name, is_unique=unique) + # only one unique task can be running - if unique and self.is_task_running(task_name): - managed_tasks_ids = list(self.get_task_group(task_name).keys()) - assert len(managed_tasks_ids) == 1 # nosec - managed_task: TrackedTask = self.get_task_group(task_name)[ - managed_tasks_ids[0] - ] + if unique and task_id in self._tracked_tasks: raise TaskAlreadyRunningError( - task_name=task_name, managed_task=managed_task + task_name=task_name, managed_task=self._tracked_tasks[task_id] ) - task_id = self.create_task_id(task_name=task_name) task_progress = TaskProgress.create(task_id=task_id) # bind the task with progress 0 and 1 @@ -402,7 +382,6 @@ async def _progress_task(progress: TaskProgress, handler: TaskProtocol): ) tracked_task = self._add_task( - task_name=task_name, task=async_task, task_progress=task_progress, task_context=task_context or {}, diff --git a/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py b/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py index ad924ec8e73..ec04392a2f6 100644 --- a/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py +++ b/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py @@ -214,11 +214,6 @@ async def test_list_tasks( list_of_tasks = TypeAdapter(list[TaskGet]).validate_python(data) assert len(list_of_tasks) == NUM_TASKS - # the task name is properly formatted - assert all( - task.task_name == "POST /long_running_task:start?num_strings=10&sleep_time=0.2" - for task in list_of_tasks - ) # now wait for them to finish await asyncio.gather(*(wait_for_task(client, task_id, {}) for task_id in results)) # now get the result one by one diff --git a/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py b/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py index bf659087e20..17c50174606 100644 --- a/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py +++ b/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py @@ -98,10 +98,9 @@ async def test_task_is_auto_removed( # meaning no calls via the manager methods are received async for attempt in AsyncRetrying(**_RETRY_PARAMS): with attempt: - for tasks in tasks_manager._tasks_groups.values(): # noqa: SLF001 - if task_id in tasks: - msg = "wait till no element is found any longer" - raise TryAgain(msg) + if task_id in tasks_manager._tracked_tasks: # noqa: SLF001 + msg = "wait till no element is found any longer" + raise TryAgain(msg) with pytest.raises(TaskNotFoundError): tasks_manager.get_task_status(task_id, with_task_context=None) @@ -171,9 +170,10 @@ async def not_unique_task(task_progress: TaskProgress): tasks_manager.start_task(task=not_unique_task) -def test_get_task_id(faker): - obj1 = TasksManager.create_task_id(faker.word()) # noqa: SLF001 - obj2 = TasksManager.create_task_id(faker.word()) # noqa: SLF001 +@pytest.mark.parametrize("is_unique", [True, False]) +def test_get_task_id(tasks_manager: TasksManager, faker: Faker, is_unique: bool): + obj1 = tasks_manager._get_task_id(faker.word(), is_unique=is_unique) # noqa: SLF001 + obj2 = tasks_manager._get_task_id(faker.word(), is_unique=is_unique) # noqa: SLF001 assert obj1 != obj2 @@ -187,7 +187,7 @@ async def test_get_status(tasks_manager: TasksManager): assert isinstance(task_status, TaskStatus) assert task_status.task_progress.message == "" assert task_status.task_progress.percent == 0.0 - assert task_status.done == False + assert task_status.done is False assert isinstance(task_status.started, datetime) @@ -372,4 +372,4 @@ async def test_define_task_name(tasks_manager: TasksManager, faker: Faker): total_sleep=10, task_name=task_name, ) - assert task_id.startswith(urllib.parse.quote(task_name, safe="")) + assert urllib.parse.quote(task_name, safe="") in task_id diff --git a/services/web/server/src/simcore_service_webserver/tasks/_rest.py b/services/web/server/src/simcore_service_webserver/tasks/_rest.py index 9b6ce8411d8..b45ff0a1260 100644 --- a/services/web/server/src/simcore_service_webserver/tasks/_rest.py +++ b/services/web/server/src/simcore_service_webserver/tasks/_rest.py @@ -78,7 +78,6 @@ async def get_async_jobs(request: web.Request) -> web.Response: [ TaskGet( task_id=f"{job.job_id}", - task_name=job.job_name, status_href=f"{request.url.with_path(str(request.app.router['get_async_job_status'].url_for(task_id=str(job.job_id))))}", abort_href=f"{request.url.with_path(str(request.app.router['cancel_async_job'].url_for(task_id=str(job.job_id))))}", result_href=f"{request.url.with_path(str(request.app.router['get_async_job_result'].url_for(task_id=str(job.job_id))))}", @@ -88,7 +87,6 @@ async def get_async_jobs(request: web.Request) -> web.Response: + [ TaskGet( task_id=f"{task.task_id}", - task_name=task.task_name, status_href=f"{request.app.router['get_task_status'].url_for(task_id=task.task_id)}", abort_href=f"{request.app.router['cancel_and_delete_task'].url_for(task_id=task.task_id)}", result_href=f"{request.app.router['get_task_result'].url_for(task_id=task.task_id)}", diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_cancellations.py b/services/web/server/tests/unit/with_dbs/02/test_projects_cancellations.py index 61353b6f624..ca6c1e2874a 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_cancellations.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_cancellations.py @@ -174,8 +174,6 @@ async def test_copying_large_project_and_retrieving_copy_task( assert not error list_of_tasks = TypeAdapter(list[TaskGet]).validate_python(data) assert len(list_of_tasks) == 1 - task = list_of_tasks[0] - assert task.task_name == f"POST {create_url}" # let the copy start await asyncio.sleep(2) # now abort the copy From d8365e8d2f9a5b63c0d7e24f0a9f3b46e66ff3a3 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Thu, 12 Jun 2025 16:36:27 +0200 Subject: [PATCH 03/33] fixed test --- .../director-v2/tests/unit/test_api_route_dynamic_scheduler.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py b/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py index 41abda858bb..73101b56baf 100644 --- a/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py +++ b/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py @@ -201,8 +201,9 @@ async def test_409_response( ) assert response.status_code == status.HTTP_202_ACCEPTED task_id = response.json() - assert task_id.startswith( + assert ( f"simcore_service_director_v2.api.routes.dynamic_scheduler.{task_name}." + in task_id ) response = client.request( From 92eb37271f84c687516a1a8a02724978e16ce0d0 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 09:21:08 +0200 Subject: [PATCH 04/33] reduce log level --- .../modules/dynamic_sidecar/docker_states.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_states.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_states.py index afd44dc0f59..f5c48cebd18 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_states.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_states.py @@ -1,6 +1,7 @@ """ States from Docker Tasks and docker Containers are mapped to ServiceState. """ + import logging from models_library.generated_models.docker_rest_api import ContainerState @@ -89,7 +90,7 @@ def extract_containers_minimum_statuses( the lowest (considered worst) state will be forwarded to the frontend. `ServiceState` defines the order of the states. """ - logger.info("containers_inspect=%s", containers_inspect) + logger.debug("containers_inspect=%s", containers_inspect) remapped_service_statuses = { index: _extract_container_status(value.container_state) for index, value in enumerate(containers_inspect) From 835c2a96b3c5028e8ddfef3a9b7148a6d10e40f2 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 09:21:40 +0200 Subject: [PATCH 05/33] refactor --- .../modules/dynamic_sidecar/docker_states.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_states.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_states.py index f5c48cebd18..7978d6d57a2 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_states.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_states.py @@ -9,7 +9,7 @@ from ...models.dynamic_services_scheduler import DockerContainerInspect -logger = logging.getLogger(__name__) +_logger = logging.getLogger(__name__) # For all available task states SEE # https://docs.docker.com/engine/swarm/how-swarm-mode-works/swarm-task-states/ @@ -63,7 +63,7 @@ def extract_task_state(task_status: dict[str, str]) -> tuple[ServiceState, str]: - last_task_error_msg = task_status["Err"] if "Err" in task_status else "" + last_task_error_msg = task_status.get("Err", "") task_state = _TASK_STATE_TO_SERVICE_STATE[task_status["State"]] return (task_state, last_task_error_msg) @@ -90,7 +90,7 @@ def extract_containers_minimum_statuses( the lowest (considered worst) state will be forwarded to the frontend. `ServiceState` defines the order of the states. """ - logger.debug("containers_inspect=%s", containers_inspect) + _logger.debug("containers_inspect=%s", containers_inspect) remapped_service_statuses = { index: _extract_container_status(value.container_state) for index, value in enumerate(containers_inspect) From 910b47c6e34eed2ba2d4033177cec6073fe8c152 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 09:31:30 +0200 Subject: [PATCH 06/33] fixed issue --- .../director-v2/tests/unit/test_api_route_dynamic_scheduler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py b/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py index 73101b56baf..37f2b7b4965 100644 --- a/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py +++ b/services/director-v2/tests/unit/test_api_route_dynamic_scheduler.py @@ -202,7 +202,7 @@ async def test_409_response( assert response.status_code == status.HTTP_202_ACCEPTED task_id = response.json() assert ( - f"simcore_service_director_v2.api.routes.dynamic_scheduler.{task_name}." + f"simcore_service_director_v2.api.routes.dynamic_scheduler.{task_name}" in task_id ) From e6cbe6642541e3a9a12129d91835aaa5fd684ba7 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 09:34:26 +0200 Subject: [PATCH 07/33] fixed test --- .../unit/test_api_rest_containers_long_running_tasks.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/services/dynamic-sidecar/tests/unit/test_api_rest_containers_long_running_tasks.py b/services/dynamic-sidecar/tests/unit/test_api_rest_containers_long_running_tasks.py index b00491ce764..9218315945f 100644 --- a/services/dynamic-sidecar/tests/unit/test_api_rest_containers_long_running_tasks.py +++ b/services/dynamic-sidecar/tests/unit/test_api_rest_containers_long_running_tasks.py @@ -508,13 +508,15 @@ def _get_awaitable() -> Awaitable: with mock_tasks(mocker): task_id = await _get_awaitable() + assert task_id.endswith("unique") async with auto_remove_task(client, task_id): assert await _get_awaitable() == task_id # since the previous task was already removed it is again possible - # to create a task + # to create a task and it will share the same task_id new_task_id = await _get_awaitable() - assert new_task_id != task_id + assert new_task_id.endswith("unique") + assert new_task_id == task_id async with auto_remove_task(client, task_id): pass From c3ab23295cf40457deaaeeb22fc56276ca76f1bd Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 09:36:07 +0200 Subject: [PATCH 08/33] fixed warnings --- .../src/servicelib/aiohttp/long_running_tasks/_server.py | 1 - .../web/server/src/simcore_service_webserver/storage/_rest.py | 2 -- 2 files changed, 3 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py index 3f0a0be3afe..eb0a4feeb01 100644 --- a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py +++ b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py @@ -77,7 +77,6 @@ async def start_long_running_task( ) task_get = TaskGet( task_id=task_id, - task_name=task_name, status_href=f"{status_url}", result_href=f"{result_url}", abort_href=f"{abort_url}", diff --git a/services/web/server/src/simcore_service_webserver/storage/_rest.py b/services/web/server/src/simcore_service_webserver/storage/_rest.py index d48d9f3a680..2c0577e0c93 100644 --- a/services/web/server/src/simcore_service_webserver/storage/_rest.py +++ b/services/web/server/src/simcore_service_webserver/storage/_rest.py @@ -183,7 +183,6 @@ def _create_data_response_from_async_job( return create_data_response( TaskGet( task_id=async_job_id, - task_name=async_job_id, status_href=f"{request.url.with_path(str(request.app.router['get_async_job_status'].url_for(task_id=async_job_id)))}", abort_href=f"{request.url.with_path(str(request.app.router['cancel_async_job'].url_for(task_id=async_job_id)))}", result_href=f"{request.url.with_path(str(request.app.router['get_async_job_result'].url_for(task_id=async_job_id)))}", @@ -503,7 +502,6 @@ def allow_only_simcore(cls, v: int) -> int: return create_data_response( TaskGet( task_id=_job_id, - task_name=_job_id, status_href=f"{request.url.with_path(str(request.app.router['get_async_job_status'].url_for(task_id=_job_id)))}", abort_href=f"{request.url.with_path(str(request.app.router['cancel_async_job'].url_for(task_id=_job_id)))}", result_href=f"{request.url.with_path(str(request.app.router['get_async_job_result'].url_for(task_id=_job_id)))}", From b33189a5817394c66ebceb3a3fc844488ea42d83 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 09:36:38 +0200 Subject: [PATCH 09/33] using dots as sperators --- .../service-library/src/servicelib/long_running_tasks/task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/service-library/src/servicelib/long_running_tasks/task.py b/packages/service-library/src/servicelib/long_running_tasks/task.py index 77ba5ba9456..ffd935f9945 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/task.py +++ b/packages/service-library/src/servicelib/long_running_tasks/task.py @@ -314,7 +314,7 @@ async def remove_task( def _get_task_id(self, task_name: str, *, is_unique: bool) -> TaskId: unique_part = "unique" if is_unique else f"{uuid4()}" - return f"{self.namespace}_{task_name}_{unique_part}" + return f"{self.namespace}.{task_name}.{unique_part}" def start_task( self, From d9be27aafc6fc6d0d34f39c7429339cf3e2a8da3 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 09:48:12 +0200 Subject: [PATCH 10/33] fixed specs --- .../server/src/simcore_service_webserver/api/v0/openapi.yaml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 73e8b2d4d7e..7ec97ca2efb 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -16716,9 +16716,6 @@ components: task_id: type: string title: Task Id - task_name: - type: string - title: Task Name status_href: type: string title: Status Href @@ -16731,7 +16728,6 @@ components: type: object required: - task_id - - task_name - status_href - result_href - abort_href From 77855dd8f3e21c6efbe8bc8944aa9b6c92767c1b Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 09:51:01 +0200 Subject: [PATCH 11/33] updated note --- .../src/servicelib/long_running_tasks/task.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/service-library/src/servicelib/long_running_tasks/task.py b/packages/service-library/src/servicelib/long_running_tasks/task.py index ffd935f9945..b03ae5b38c7 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/task.py +++ b/packages/service-library/src/servicelib/long_running_tasks/task.py @@ -27,9 +27,9 @@ _logger = logging.getLogger(__name__) -_DEFAULT_NAMESPACE: Final = ( - "lrt" # NOTE: for now only one is used, will change in future PRs -) +# NOTE: for now only this one is used, in future it will be unqiue per service +# and this default will be removed and become mandatory +_DEFAULT_NAMESPACE: Final = "lrt" _CANCEL_TASK_TIMEOUT: Final[PositiveFloat] = datetime.timedelta( seconds=1 From 01c58db991dd5f985bf621d0e11507d654f7809e Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 10:49:13 +0200 Subject: [PATCH 12/33] refactor to use registered task --- .../aiohttp/long_running_tasks/_server.py | 6 +- .../servicelib/long_running_tasks/errors.py | 7 ++ .../src/servicelib/long_running_tasks/task.py | 23 +++++- .../aiohttp/long_running_tasks/conftest.py | 11 ++- .../test_long_running_tasks.py | 2 +- ...st_long_running_tasks_with_task_context.py | 2 +- .../test_long_running_tasks.py | 23 +++--- ...test_long_running_tasks_context_manager.py | 19 ++++- .../test_long_running_tasks_task.py | 77 ++++++++++------- .../api/routes/dynamic_scheduler.py | 37 ++++++--- .../api/rest/containers_long_running_tasks.py | 18 ++-- .../modules/long_running_tasks.py | 82 +++++++++---------- .../projects/_controller/nodes_rest.py | 9 +- .../projects/_controller/projects_rest.py | 4 +- .../projects/_crud_api_create.py | 16 ++-- 15 files changed, 210 insertions(+), 126 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py index eb0a4feeb01..a4b77c7d4be 100644 --- a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py +++ b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py @@ -14,7 +14,7 @@ DEFAULT_STALE_TASK_DETECT_TIMEOUT, ) from ...long_running_tasks.models import TaskGet -from ...long_running_tasks.task import TaskContext, TaskProtocol +from ...long_running_tasks.task import RegisteredTaskName, TaskContext from ..typing_extension import Handler from . import _routes from ._constants import ( @@ -45,7 +45,7 @@ def _create_task_name_from_request(request: web.Request) -> str: async def start_long_running_task( # NOTE: positional argument are suffixed with "_" to avoid name conflicts with "task_kwargs" keys request_: web.Request, - task_: TaskProtocol, + registerd_task_name: RegisteredTaskName, *, fire_and_forget: bool = False, task_context: TaskContext, @@ -56,7 +56,7 @@ async def start_long_running_task( task_id = None try: task_id = long_running_manager.tasks_manager.start_task( - task_, + registerd_task_name, fire_and_forget=fire_and_forget, task_context=task_context, task_name=task_name, diff --git a/packages/service-library/src/servicelib/long_running_tasks/errors.py b/packages/service-library/src/servicelib/long_running_tasks/errors.py index 33439c6436f..e199a3128ce 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/errors.py +++ b/packages/service-library/src/servicelib/long_running_tasks/errors.py @@ -5,6 +5,13 @@ class BaseLongRunningError(OsparcErrorMixin, Exception): """base exception for this module""" +class TaskNotRegisteredError(BaseLongRunningError): + msg_template: str = ( + "notask with task_name='{task_name}' was found in the task registry. " + "Make sure it's registered before starting it." + ) + + class TaskAlreadyRunningError(BaseLongRunningError): msg_template: str = "{task_name} must be unique, found: '{managed_task}'" diff --git a/packages/service-library/src/servicelib/long_running_tasks/task.py b/packages/service-library/src/servicelib/long_running_tasks/task.py index b03ae5b38c7..5bd6bbe0ca5 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/task.py +++ b/packages/service-library/src/servicelib/long_running_tasks/task.py @@ -6,7 +6,7 @@ import urllib.parse from collections import deque from contextlib import suppress -from typing import Any, Final, Protocol, TypeAlias +from typing import Any, ClassVar, Final, Protocol, TypeAlias from uuid import uuid4 from models_library.api_schemas_long_running_tasks.base import TaskProgress @@ -21,6 +21,7 @@ TaskExceptionError, TaskNotCompletedError, TaskNotFoundError, + TaskNotRegisteredError, ) from .models import TaskId, TaskStatus, TrackedTask @@ -35,6 +36,7 @@ seconds=1 ).total_seconds() +RegisteredTaskName: TypeAlias = str Namespace: TypeAlias = str TrackedTaskGroupDict: TypeAlias = dict[TaskId, TrackedTask] TaskContext: TypeAlias = dict[str, Any] @@ -49,6 +51,19 @@ async def __call__( def __name__(self) -> str: ... +class TaskRegistry: + REGISTERED_TASKS: ClassVar[dict[RegisteredTaskName, TaskProtocol]] = {} + + @classmethod + def register(cls, task: TaskProtocol) -> None: + cls.REGISTERED_TASKS[task.__name__] = task + + @classmethod + def unregister(cls, task: TaskProtocol) -> None: + if task.__name__ in cls.REGISTERED_TASKS: + del cls.REGISTERED_TASKS[task.__name__] + + async def _await_task(task: asyncio.Task) -> None: await task @@ -318,7 +333,7 @@ def _get_task_id(self, task_name: str, *, is_unique: bool) -> TaskId: def start_task( self, - task: TaskProtocol, + registered_task_name: RegisteredTaskName, *, unique: bool = False, task_context: TaskContext | None = None, @@ -351,6 +366,10 @@ def start_task( Returns: TaskId: the task unique identifier """ + if registered_task_name not in TaskRegistry.REGISTERED_TASKS: + raise TaskNotRegisteredError(task_name=registered_task_name) + + task = TaskRegistry.REGISTERED_TASKS[registered_task_name] # NOTE: If not task name is given, it will be composed of the handler's module and it's name # to keep the urls shorter and more meaningful. diff --git a/packages/service-library/tests/aiohttp/long_running_tasks/conftest.py b/packages/service-library/tests/aiohttp/long_running_tasks/conftest.py index 4f39c80be08..bd8d77da3ce 100644 --- a/packages/service-library/tests/aiohttp/long_running_tasks/conftest.py +++ b/packages/service-library/tests/aiohttp/long_running_tasks/conftest.py @@ -19,7 +19,7 @@ TaskProgress, TaskStatus, ) -from servicelib.long_running_tasks.task import TaskContext +from servicelib.long_running_tasks.task import TaskContext, TaskRegistry from tenacity.asyncio import AsyncRetrying from tenacity.retry import retry_if_exception_type from tenacity.stop import stop_after_delay @@ -27,7 +27,7 @@ async def _string_list_task( - task_progress: TaskProgress, + progress: TaskProgress, num_strings: int, sleep_time: float, fail: bool, @@ -36,7 +36,7 @@ async def _string_list_task( for index in range(num_strings): generated_strings.append(f"{index}") await asyncio.sleep(sleep_time) - task_progress.update(message="generated item", percent=index / num_strings) + progress.update(message="generated item", percent=index / num_strings) if fail: msg = "We were asked to fail!!" raise RuntimeError(msg) @@ -47,6 +47,9 @@ async def _string_list_task( ) +TaskRegistry.register(_string_list_task) + + @pytest.fixture def task_context(faker: Faker) -> TaskContext: return {"user_id": faker.pyint(), "product": faker.pystr()} @@ -73,7 +76,7 @@ async def generate_list_strings(request: web.Request) -> web.Response: query_params = parse_request_query_parameters_as(_LongTaskQueryParams, request) return await long_running_tasks.server.start_long_running_task( request, - _string_list_task, + _string_list_task.__name__, num_strings=query_params.num_strings, sleep_time=query_params.sleep_time, fail=query_params.fail, diff --git a/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py b/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py index ec04392a2f6..6d215e0ae8a 100644 --- a/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py +++ b/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py @@ -97,7 +97,7 @@ async def test_workflow( # now get the result result_url = client.app.router["get_task_result"].url_for(task_id=task_id) result = await client.get(f"{result_url}") - task_result, error = await assert_status(result, status.HTTP_201_CREATED) + task_result, error = await assert_status(result, status.HTTP_200_OK) assert task_result assert not error assert task_result == [f"{x}" for x in range(10)] diff --git a/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks_with_task_context.py b/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks_with_task_context.py index 20f8dbf657f..8f7ff5efd23 100644 --- a/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks_with_task_context.py +++ b/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks_with_task_context.py @@ -151,7 +151,7 @@ async def test_get_task_result( await assert_status(resp, status.HTTP_404_NOT_FOUND) # calling with context should find the task resp = await client_with_task_context.get(f"{result_url.with_query(task_context)}") - await assert_status(resp, status.HTTP_201_CREATED) + await assert_status(resp, status.HTTP_200_OK) async def test_cancel_task( diff --git a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py index 027bac7b74a..428d1d3ab8c 100644 --- a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py +++ b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py @@ -12,7 +12,7 @@ import asyncio import json from collections.abc import AsyncIterator, Awaitable, Callable -from typing import Final +from typing import Annotated, Final import pytest from asgi_lifespan import LifespanManager @@ -31,7 +31,7 @@ TaskProgress, TaskStatus, ) -from servicelib.long_running_tasks.task import TaskContext +from servicelib.long_running_tasks.task import TaskContext, TaskRegistry from tenacity.asyncio import AsyncRetrying from tenacity.retry import retry_if_exception_type from tenacity.stop import stop_after_delay @@ -42,7 +42,7 @@ async def _string_list_task( - task_progress: TaskProgress, + progress: TaskProgress, num_strings: int, sleep_time: float, fail: bool, @@ -51,7 +51,7 @@ async def _string_list_task( for index in range(num_strings): generated_strings.append(f"{index}") await asyncio.sleep(sleep_time) - task_progress.update(message="generated item", percent=index / num_strings) + progress.update(message="generated item", percent=index / num_strings) if fail: msg = "We were asked to fail!!" raise RuntimeError(msg) @@ -59,6 +59,9 @@ async def _string_list_task( return generated_strings +TaskRegistry.register(_string_list_task) + + @pytest.fixture def server_routes() -> APIRouter: routes = APIRouter() @@ -69,18 +72,18 @@ def server_routes() -> APIRouter: async def create_string_list_task( num_strings: int, sleep_time: float, + long_running_manager: Annotated[ + FastAPILongRunningManager, Depends(get_long_running_manager) + ], + *, fail: bool = False, - long_running_manager: FastAPILongRunningManager = Depends( - get_long_running_manager - ), ) -> TaskId: - task_id = long_running_manager.tasks_manager.start_task( - _string_list_task, + return long_running_manager.tasks_manager.start_task( + _string_list_task.__name__, num_strings=num_strings, sleep_time=sleep_time, fail=fail, ) - return task_id return routes diff --git a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py index 02c2916f6e9..e0febe73707 100644 --- a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py +++ b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py @@ -25,6 +25,7 @@ TaskId, TaskProgress, ) +from servicelib.long_running_tasks.task import TaskRegistry TASK_SLEEP_INTERVAL: Final[PositiveFloat] = 0.1 @@ -38,17 +39,25 @@ async def _assert_task_removed( assert result.status_code == status.HTTP_404_NOT_FOUND -async def a_test_task(task_progress: TaskProgress) -> int: +async def a_test_task(progress: TaskProgress) -> int: + _ = progress await asyncio.sleep(TASK_SLEEP_INTERVAL) return 42 -async def a_failing_test_task(task_progress: TaskProgress) -> None: +TaskRegistry.register(a_test_task) + + +async def a_failing_test_task(progress: TaskProgress) -> None: + _ = progress await asyncio.sleep(TASK_SLEEP_INTERVAL) msg = "I am failing as requested" raise RuntimeError(msg) +TaskRegistry.register(a_failing_test_task) + + @pytest.fixture def user_routes() -> APIRouter: router = APIRouter() @@ -59,7 +68,7 @@ async def create_task_user_defined_route( FastAPILongRunningManager, Depends(get_long_running_manager) ], ) -> TaskId: - return long_running_manager.tasks_manager.start_task(task=a_test_task) + return long_running_manager.tasks_manager.start_task(a_test_task.__name__) @router.get("/api/failing", status_code=status.HTTP_200_OK) async def create_task_which_fails( @@ -67,7 +76,9 @@ async def create_task_which_fails( FastAPILongRunningManager, Depends(get_long_running_manager) ], ) -> TaskId: - return long_running_manager.tasks_manager.start_task(task=a_failing_test_task) + return long_running_manager.tasks_manager.start_task( + a_failing_test_task.__name__ + ) return router diff --git a/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py b/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py index 17c50174606..2e32bfde908 100644 --- a/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py +++ b/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py @@ -17,13 +17,10 @@ TaskCancelledError, TaskNotCompletedError, TaskNotFoundError, + TaskNotRegisteredError, ) -from servicelib.long_running_tasks.models import ( - ProgressPercent, - TaskProgress, - TaskStatus, -) -from servicelib.long_running_tasks.task import TasksManager +from servicelib.long_running_tasks.models import TaskProgress, TaskStatus +from servicelib.long_running_tasks.task import TaskRegistry, TasksManager from tenacity import TryAgain from tenacity.asyncio import AsyncRetrying from tenacity.retry import retry_if_exception_type @@ -39,14 +36,14 @@ async def a_background_task( - task_progress: TaskProgress, + progress: TaskProgress, raise_when_finished: bool, total_sleep: int, ) -> int: """sleeps and raises an error or returns 42""" for i in range(total_sleep): await asyncio.sleep(1) - task_progress.update(percent=ProgressPercent((i + 1) / total_sleep)) + progress.update(percent=(i + 1) / total_sleep) if raise_when_finished: msg = "raised this error as instructed" raise RuntimeError(msg) @@ -54,17 +51,21 @@ async def a_background_task( return 42 -async def fast_background_task(task_progress: TaskProgress) -> int: +async def fast_background_task(progress: TaskProgress) -> int: """this task does nothing and returns a constant""" return 42 -async def failing_background_task(task_progress: TaskProgress): +async def failing_background_task(progress: TaskProgress): """this task does nothing and returns a constant""" msg = "failing asap" raise RuntimeError(msg) +TaskRegistry.register(a_background_task) +TaskRegistry.register(fast_background_task) +TaskRegistry.register(failing_background_task) + TEST_CHECK_STALE_INTERVAL_S: Final[float] = 1 @@ -84,7 +85,7 @@ async def test_task_is_auto_removed( tasks_manager: TasksManager, check_task_presence_before: bool ): task_id = tasks_manager.start_task( - a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10 * TEST_CHECK_STALE_INTERVAL_S, ) @@ -110,7 +111,7 @@ async def test_task_is_auto_removed( async def test_checked_task_is_not_auto_removed(tasks_manager: TasksManager): task_id = tasks_manager.start_task( - a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=5 * TEST_CHECK_STALE_INTERVAL_S, ) @@ -124,7 +125,7 @@ async def test_checked_task_is_not_auto_removed(tasks_manager: TasksManager): async def test_fire_and_forget_task_is_not_auto_removed(tasks_manager: TasksManager): task_id = tasks_manager.start_task( - a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=5 * TEST_CHECK_STALE_INTERVAL_S, fire_and_forget=True, @@ -142,7 +143,7 @@ async def test_fire_and_forget_task_is_not_auto_removed(tasks_manager: TasksMana async def test_get_result_of_unfinished_task_raises(tasks_manager: TasksManager): task_id = tasks_manager.start_task( - a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=5 * TEST_CHECK_STALE_INTERVAL_S, ) @@ -151,23 +152,32 @@ async def test_get_result_of_unfinished_task_raises(tasks_manager: TasksManager) async def test_unique_task_already_running(tasks_manager: TasksManager): - async def unique_task(task_progress: TaskProgress): + async def unique_task(progress: TaskProgress): + _ = progress await asyncio.sleep(1) - tasks_manager.start_task(task=unique_task, unique=True) + TaskRegistry.register(unique_task) + + tasks_manager.start_task(unique_task.__name__, unique=True) # ensure unique running task regardless of how many times it gets started with pytest.raises(TaskAlreadyRunningError) as exec_info: - tasks_manager.start_task(task=unique_task, unique=True) + tasks_manager.start_task(unique_task.__name__, unique=True) assert "must be unique, found: " in f"{exec_info.value}" + TaskRegistry.unregister(unique_task) + async def test_start_multiple_not_unique_tasks(tasks_manager: TasksManager): - async def not_unique_task(task_progress: TaskProgress): + async def not_unique_task(progress: TaskProgress): await asyncio.sleep(1) + TaskRegistry.register(not_unique_task) + for _ in range(5): - tasks_manager.start_task(task=not_unique_task) + tasks_manager.start_task(not_unique_task.__name__) + + TaskRegistry.unregister(not_unique_task) @pytest.mark.parametrize("is_unique", [True, False]) @@ -179,7 +189,7 @@ def test_get_task_id(tasks_manager: TasksManager, faker: Faker, is_unique: bool) async def test_get_status(tasks_manager: TasksManager): task_id = tasks_manager.start_task( - task=a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10, ) @@ -198,7 +208,7 @@ async def test_get_status_missing(tasks_manager: TasksManager): async def test_get_result(tasks_manager: TasksManager): - task_id = tasks_manager.start_task(task=fast_background_task) + task_id = tasks_manager.start_task(fast_background_task.__name__) await asyncio.sleep(0.1) result = tasks_manager.get_task_result(task_id, with_task_context=None) assert result == 42 @@ -211,7 +221,7 @@ async def test_get_result_missing(tasks_manager: TasksManager): async def test_get_result_finished_with_error(tasks_manager: TasksManager): - task_id = tasks_manager.start_task(task=failing_background_task) + task_id = tasks_manager.start_task(failing_background_task.__name__) # wait for result async for attempt in AsyncRetrying(**_RETRY_PARAMS): with attempt: @@ -225,7 +235,7 @@ async def test_get_result_task_was_cancelled_multiple_times( tasks_manager: TasksManager, ): task_id = tasks_manager.start_task( - task=a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10, ) @@ -240,7 +250,7 @@ async def test_get_result_task_was_cancelled_multiple_times( async def test_remove_task(tasks_manager: TasksManager): task_id = tasks_manager.start_task( - task=a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10, ) @@ -255,7 +265,7 @@ async def test_remove_task(tasks_manager: TasksManager): async def test_remove_task_with_task_context(tasks_manager: TasksManager): TASK_CONTEXT = {"some_context": "some_value"} task_id = tasks_manager.start_task( - task=a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10, task_context=TASK_CONTEXT, @@ -287,7 +297,7 @@ async def test_remove_unknown_task(tasks_manager: TasksManager): async def test_cancel_task_with_task_context(tasks_manager: TasksManager): TASK_CONTEXT = {"some_context": "some_value"} task_id = tasks_manager.start_task( - task=a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10, task_context=TASK_CONTEXT, @@ -313,7 +323,7 @@ async def test_list_tasks(tasks_manager: TasksManager): for _ in range(NUM_TASKS): task_ids.append( # noqa: PERF401 tasks_manager.start_task( - task=a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10, ) @@ -328,18 +338,18 @@ async def test_list_tasks(tasks_manager: TasksManager): async def test_list_tasks_filtering(tasks_manager: TasksManager): tasks_manager.start_task( - task=a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10, ) tasks_manager.start_task( - task=a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10, task_context={"user_id": 213}, ) tasks_manager.start_task( - task=a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10, task_context={"user_id": 213, "product": "osparc"}, @@ -367,9 +377,14 @@ async def test_list_tasks_filtering(tasks_manager: TasksManager): async def test_define_task_name(tasks_manager: TasksManager, faker: Faker): task_name = faker.name() task_id = tasks_manager.start_task( - task=a_background_task, + a_background_task.__name__, raise_when_finished=False, total_sleep=10, task_name=task_name, ) assert urllib.parse.quote(task_name, safe="") in task_id + + +async def test_start_not_registered_task(tasks_manager: TasksManager): + with pytest.raises(TaskNotRegisteredError): + tasks_manager.start_task("not_registered_task") diff --git a/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py b/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py index c947cf5bca6..7ccc50376d3 100644 --- a/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py +++ b/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py @@ -13,6 +13,7 @@ TaskId, TaskProgress, ) +from servicelib.long_running_tasks.task import TaskRegistry from tenacity import retry from tenacity.before_sleep import before_sleep_log from tenacity.retry import retry_if_result @@ -99,25 +100,29 @@ async def delete_service_containers( ], ): async def _task_remove_service_containers( - task_progress: TaskProgress, node_uuid: NodeID + progress: TaskProgress, node_uuid: NodeID ) -> None: async def _progress_callback( message: ProgressMessage, percent: ProgressPercent | None, _: TaskId ) -> None: - task_progress.update(message=message, percent=percent) + progress.update(message=message, percent=percent) await dynamic_sidecars_scheduler.remove_service_containers( node_uuid=node_uuid, progress_callback=_progress_callback ) + TaskRegistry.register(_task_remove_service_containers) + try: return long_running_manager.tasks_manager.start_task( - task=_task_remove_service_containers, # type: ignore[arg-type] + _task_remove_service_containers.__name__, unique=True, node_uuid=node_uuid, ) except TaskAlreadyRunningError as e: raise HTTPException(status.HTTP_409_CONFLICT, detail=f"{e}") from e + finally: + TaskRegistry.unregister(_task_remove_service_containers) @router.get( @@ -158,26 +163,30 @@ async def save_service_state( ], ): async def _task_save_service_state( - task_progress: TaskProgress, + progress: TaskProgress, node_uuid: NodeID, ) -> None: async def _progress_callback( message: ProgressMessage, percent: ProgressPercent | None, _: TaskId ) -> None: - task_progress.update(message=message, percent=percent) + progress.update(message=message, percent=percent) await dynamic_sidecars_scheduler.save_service_state( node_uuid=node_uuid, progress_callback=_progress_callback ) + TaskRegistry.register(_task_save_service_state) + try: return long_running_manager.tasks_manager.start_task( - task=_task_save_service_state, # type: ignore[arg-type] + _task_save_service_state.__name__, unique=True, node_uuid=node_uuid, ) except TaskAlreadyRunningError as e: raise HTTPException(status.HTTP_409_CONFLICT, detail=f"{e}") from e + finally: + TaskRegistry.unregister(_task_save_service_state) @router.post( @@ -201,17 +210,19 @@ async def push_service_outputs( ], ): async def _task_push_service_outputs( - task_progress: TaskProgress, node_uuid: NodeID + progress: TaskProgress, node_uuid: NodeID ) -> None: async def _progress_callback( message: ProgressMessage, percent: ProgressPercent | None, _: TaskId ) -> None: - task_progress.update(message=message, percent=percent) + progress.update(message=message, percent=percent) await dynamic_sidecars_scheduler.push_service_outputs( node_uuid=node_uuid, progress_callback=_progress_callback ) + TaskRegistry.register(_task_push_service_outputs) + try: return long_running_manager.tasks_manager.start_task( task=_task_push_service_outputs, # type: ignore[arg-type] @@ -220,6 +231,8 @@ async def _progress_callback( ) except TaskAlreadyRunningError as e: raise HTTPException(status.HTTP_409_CONFLICT, detail=f"{e}") from e + finally: + TaskRegistry.unregister(_task_push_service_outputs) @router.delete( @@ -243,12 +256,14 @@ async def delete_service_docker_resources( ], ): async def _task_cleanup_service_docker_resources( - task_progress: TaskProgress, node_uuid: NodeID + progress: TaskProgress, node_uuid: NodeID ) -> None: await dynamic_sidecars_scheduler.remove_service_sidecar_proxy_docker_networks_and_volumes( - task_progress=task_progress, node_uuid=node_uuid + task_progress=progress, node_uuid=node_uuid ) + TaskRegistry.register(_task_cleanup_service_docker_resources) + try: return long_running_manager.tasks_manager.start_task( task=_task_cleanup_service_docker_resources, # type: ignore[arg-type] @@ -257,6 +272,8 @@ async def _task_cleanup_service_docker_resources( ) except TaskAlreadyRunningError as e: raise HTTPException(status.HTTP_409_CONFLICT, detail=f"{e}") from e + finally: + TaskRegistry.unregister(_task_cleanup_service_docker_resources) @router.post( diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py index 0b97b6f5027..14d6f8cdbbd 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py @@ -58,7 +58,7 @@ async def pull_user_servcices_docker_images( try: return long_running_manager.tasks_manager.start_task( - task=task_pull_user_servcices_docker_images, + task_pull_user_servcices_docker_images.__name__, unique=True, app=app, shared_store=shared_store, @@ -98,7 +98,7 @@ async def create_service_containers_task( # pylint: disable=too-many-arguments try: return long_running_manager.tasks_manager.start_task( - task=task_create_service_containers, + task_create_service_containers.__name__, unique=True, settings=settings, containers_create=containers_create, @@ -131,7 +131,7 @@ async def runs_docker_compose_down_task( try: return long_running_manager.tasks_manager.start_task( - task=task_runs_docker_compose_down, + task_runs_docker_compose_down.__name__, unique=True, app=app, shared_store=shared_store, @@ -162,7 +162,7 @@ async def state_restore_task( try: return long_running_manager.tasks_manager.start_task( - task=task_restore_state, + task_restore_state.__name__, unique=True, settings=settings, mounted_volumes=mounted_volumes, @@ -192,7 +192,7 @@ async def state_save_task( try: return long_running_manager.tasks_manager.start_task( - task=task_save_state, + task_save_state.__name__, unique=True, settings=settings, mounted_volumes=mounted_volumes, @@ -224,7 +224,7 @@ async def ports_inputs_pull_task( try: return long_running_manager.tasks_manager.start_task( - task=task_ports_inputs_pull, + task_ports_inputs_pull.__name__, unique=True, port_keys=port_keys, mounted_volumes=mounted_volumes, @@ -256,7 +256,7 @@ async def ports_outputs_pull_task( try: return long_running_manager.tasks_manager.start_task( - task=task_ports_outputs_pull, + task_ports_outputs_pull.__name__, unique=True, port_keys=port_keys, mounted_volumes=mounted_volumes, @@ -285,7 +285,7 @@ async def ports_outputs_push_task( try: return long_running_manager.tasks_manager.start_task( - task=task_ports_outputs_push, + task_ports_outputs_push.__name__, unique=True, outputs_manager=outputs_manager, app=app, @@ -314,7 +314,7 @@ async def containers_restart_task( try: return long_running_manager.tasks_manager.start_task( - task=task_containers_restart, + task_containers_restart.__name__, unique=True, app=app, settings=settings, diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/long_running_tasks.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/long_running_tasks.py index 42412376d08..f14f90be1ef 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/long_running_tasks.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/modules/long_running_tasks.py @@ -6,16 +6,14 @@ from typing import Final from fastapi import FastAPI -from models_library.api_schemas_long_running_tasks.base import ( - ProgressPercent, - TaskProgress, -) +from models_library.api_schemas_long_running_tasks.base import TaskProgress from models_library.generated_models.docker_rest_api import ContainerState from models_library.rabbitmq_messages import ProgressType, SimcorePlatformStatus from models_library.service_settings_labels import LegacyState from pydantic import PositiveInt from servicelib.file_utils import log_directory_changes from servicelib.logging_utils import log_context +from servicelib.long_running_tasks.task import TaskRegistry from servicelib.progress_bar import ProgressBarData from servicelib.utils import logged_gather from simcore_sdk.node_data import data_manager @@ -151,13 +149,11 @@ async def task_pull_user_servcices_docker_images( ) -> None: assert shared_store.compose_spec # nosec - progress.update(message="started pulling user services", percent=ProgressPercent(0)) + progress.update(message="started pulling user services", percent=0) await docker_compose_pull(app, shared_store.compose_spec) - progress.update( - message="finished pulling user services", percent=ProgressPercent(1) - ) + progress.update(message="finished pulling user services", percent=1) async def task_create_service_containers( @@ -168,7 +164,7 @@ async def task_create_service_containers( app: FastAPI, application_health: ApplicationHealth, ) -> list[str]: - progress.update(message="validating service spec", percent=ProgressPercent(0)) + progress.update(message="validating service spec", percent=0) assert shared_store.compose_spec # nosec @@ -194,18 +190,14 @@ async def task_create_service_containers( _raise_for_errors(result, "rm") await progress_bar.update() - progress.update( - message="creating and starting containers", percent=ProgressPercent(0.90) - ) + progress.update(message="creating and starting containers", percent=0.90) await post_sidecar_log_message( app, "starting service containers", log_level=logging.INFO ) await _retry_docker_compose_create(shared_store.compose_spec, settings) await progress_bar.update() - progress.update( - message="ensure containers are started", percent=ProgressPercent(0.95) - ) + progress.update(message="ensure containers are started", percent=0.95) compose_start_result = await _retry_docker_compose_start( shared_store.compose_spec, settings ) @@ -288,9 +280,7 @@ async def _send_resource_tracking_stop(platform_status: SimcorePlatformStatus): await send_service_stopped(app, simcore_platform_status) try: - progress.update( - message="running docker-compose-down", percent=ProgressPercent(0.1) - ) + progress.update(message="running docker-compose-down", percent=0.1) await run_before_shutdown_actions( shared_store, settings.DY_SIDECAR_CALLBACKS_MAPPING.before_shutdown @@ -303,13 +293,11 @@ async def _send_resource_tracking_stop(platform_status: SimcorePlatformStatus): result = await _retry_docker_compose_down(shared_store.compose_spec, settings) _raise_for_errors(result, "down") - progress.update(message="stopping logs", percent=ProgressPercent(0.9)) + progress.update(message="stopping logs", percent=0.9) for container_name in shared_store.container_names: await stop_log_fetching(app, container_name) - progress.update( - message="removing pending resources", percent=ProgressPercent(0.95) - ) + progress.update(message="removing pending resources", percent=0.95) result = await docker_compose_rm(shared_store.compose_spec, settings) _raise_for_errors(result, "rm") except Exception: @@ -326,7 +314,7 @@ async def _send_resource_tracking_stop(platform_status: SimcorePlatformStatus): async with shared_store: shared_store.compose_spec = None shared_store.container_names = [] - progress.update(message="done", percent=ProgressPercent(0.99)) + progress.update(message="done", percent=0.99) def _get_satate_folders_size(paths: list[Path]) -> int: @@ -389,7 +377,7 @@ async def task_restore_state( # NOTE: this implies that the legacy format will always be decompressed # until it is not removed. - progress.update(message="Downloading state", percent=ProgressPercent(0.05)) + progress.update(message="Downloading state", percent=0.05) state_paths = list(mounted_volumes.disk_state_paths_iter()) await post_sidecar_log_message( app, @@ -419,7 +407,7 @@ async def task_restore_state( await post_sidecar_log_message( app, "Finished state downloading", log_level=logging.INFO ) - progress.update(message="state restored", percent=ProgressPercent(0.99)) + progress.update(message="state restored", percent=0.99) return _get_satate_folders_size(state_paths) @@ -459,7 +447,7 @@ async def task_save_state( If a legacy archive is detected, it will be removed after saving the new format. """ - progress.update(message="starting state save", percent=ProgressPercent(0.0)) + progress.update(message="starting state save", percent=0.0) state_paths = list(mounted_volumes.disk_state_paths_iter()) async with ProgressBarData( num_steps=len(state_paths), @@ -485,7 +473,7 @@ async def task_save_state( ) await post_sidecar_log_message(app, "Finished state saving", log_level=logging.INFO) - progress.update(message="finished state saving", percent=ProgressPercent(0.99)) + progress.update(message="finished state saving", percent=0.99) return _get_satate_folders_size(state_paths) @@ -503,12 +491,12 @@ async def task_ports_inputs_pull( _logger.info("Received request to pull inputs but was ignored") return 0 - progress.update(message="starting inputs pulling", percent=ProgressPercent(0.0)) + progress.update(message="starting inputs pulling", percent=0.0) port_keys = [] if port_keys is None else port_keys await post_sidecar_log_message( app, f"Pulling inputs for {port_keys}", log_level=logging.INFO ) - progress.update(message="pulling inputs", percent=ProgressPercent(0.1)) + progress.update(message="pulling inputs", percent=0.1) async with ProgressBarData( num_steps=1, progress_report_cb=functools.partial( @@ -539,7 +527,7 @@ async def task_ports_inputs_pull( await post_sidecar_log_message( app, "Finished pulling inputs", log_level=logging.INFO ) - progress.update(message="finished inputs pulling", percent=ProgressPercent(0.99)) + progress.update(message="finished inputs pulling", percent=0.99) return int(transferred_bytes) @@ -549,7 +537,7 @@ async def task_ports_outputs_pull( mounted_volumes: MountedVolumes, app: FastAPI, ) -> int: - progress.update(message="starting outputs pulling", percent=ProgressPercent(0.0)) + progress.update(message="starting outputs pulling", percent=0.0) port_keys = [] if port_keys is None else port_keys await post_sidecar_log_message( app, f"Pulling output for {port_keys}", log_level=logging.INFO @@ -576,14 +564,14 @@ async def task_ports_outputs_pull( await post_sidecar_log_message( app, "Finished pulling outputs", log_level=logging.INFO ) - progress.update(message="finished outputs pulling", percent=ProgressPercent(0.99)) + progress.update(message="finished outputs pulling", percent=0.99) return int(transferred_bytes) async def task_ports_outputs_push( progress: TaskProgress, outputs_manager: OutputsManager, app: FastAPI ) -> None: - progress.update(message="starting outputs pushing", percent=ProgressPercent(0.0)) + progress.update(message="starting outputs pushing", percent=0.0) await post_sidecar_log_message( app, f"waiting for outputs {outputs_manager.outputs_context.file_type_port_keys} to be pushed", @@ -595,7 +583,7 @@ async def task_ports_outputs_push( await post_sidecar_log_message( app, "finished outputs pushing", log_level=logging.INFO ) - progress.update(message="finished outputs pushing", percent=ProgressPercent(0.99)) + progress.update(message="finished outputs pushing", percent=0.99) async def task_containers_restart( @@ -610,9 +598,7 @@ async def task_containers_restart( # or some other state, the service will get shutdown, to prevent this # blocking status while containers are being restarted. async with app.state.container_restart_lock: - progress.update( - message="starting containers restart", percent=ProgressPercent(0.0) - ) + progress.update(message="starting containers restart", percent=0.0) if shared_store.compose_spec is None: msg = "No spec for docker compose command was found" raise RuntimeError(msg) @@ -620,20 +606,34 @@ async def task_containers_restart( for container_name in shared_store.container_names: await stop_log_fetching(app, container_name) - progress.update(message="stopped log fetching", percent=ProgressPercent(0.1)) + progress.update(message="stopped log fetching", percent=0.1) result = await docker_compose_restart(shared_store.compose_spec, settings) _raise_for_errors(result, "restart") - progress.update(message="containers restarted", percent=ProgressPercent(0.8)) + progress.update(message="containers restarted", percent=0.8) for container_name in shared_store.container_names: await start_log_fetching(app, container_name) - progress.update(message="started log fetching", percent=ProgressPercent(0.9)) + progress.update(message="started log fetching", percent=0.9) await post_sidecar_log_message( app, "Service was restarted please reload the UI", log_level=logging.INFO ) await post_event_reload_iframe(app) - progress.update(message="started log fetching", percent=ProgressPercent(0.99)) + progress.update(message="started log fetching", percent=0.99) + + +for task in ( + task_pull_user_servcices_docker_images, + task_create_service_containers, + task_runs_docker_compose_down, + task_restore_state, + task_save_state, + task_ports_inputs_pull, + task_ports_outputs_pull, + task_ports_outputs_push, + task_containers_restart, +): + TaskRegistry.register(task) diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/nodes_rest.py b/services/web/server/src/simcore_service_webserver/projects/_controller/nodes_rest.py index 4e556e95d22..299756ae623 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/nodes_rest.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/nodes_rest.py @@ -43,6 +43,7 @@ X_SIMCORE_USER_AGENT, ) from servicelib.long_running_tasks.models import TaskProgress +from servicelib.long_running_tasks.task import TaskRegistry from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from servicelib.rabbitmq import RPCServerError from servicelib.rabbitmq.rpc_interfaces.dynamic_scheduler.errors import ( @@ -289,11 +290,12 @@ async def start_node(request: web.Request) -> web.Response: async def _stop_dynamic_service_task( - _task_progress: TaskProgress, + progress: TaskProgress, *, app: web.Application, dynamic_service_stop: DynamicServiceStop, ): + _ = progress # NOTE: _handle_project_nodes_exceptions only decorate handlers try: await dynamic_scheduler_service.stop_dynamic_service( @@ -310,6 +312,9 @@ async def _stop_dynamic_service_task( return web.json_response(status=status.HTTP_204_NO_CONTENT) +TaskRegistry.register(_stop_dynamic_service_task) + + @routes.post( f"/{VTAG}/projects/{{project_id}}/nodes/{{node_id}}:stop", name="stop_node" ) @@ -334,7 +339,7 @@ async def stop_node(request: web.Request) -> web.Response: return await start_long_running_task( request, - _stop_dynamic_service_task, # type: ignore[arg-type] # @GitHK, @pcrespov this one I don't know how to fix + _stop_dynamic_service_task.__name__, task_context=jsonable_encoder(req_ctx), # task arguments from here on --- app=request.app, diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py index 9fcfc3cad88..97572f18741 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/projects_rest.py @@ -97,7 +97,7 @@ async def create_project(request: web.Request): return await start_long_running_task( request, - _crud_api_create.create_project, # type: ignore[arg-type] # @GitHK, @pcrespov this one I don't know how to fix + _crud_api_create.create_project.__name__, fire_and_forget=True, task_context=jsonable_encoder(req_ctx), # arguments @@ -412,7 +412,7 @@ async def clone_project(request: web.Request): return await start_long_running_task( request, - _crud_api_create.create_project, # type: ignore[arg-type] # @GitHK, @pcrespov this one I don't know how to fix + _crud_api_create.create_project.__name__, fire_and_forget=True, task_context=jsonable_encoder(req_ctx), # arguments diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py index 04671fdd21b..b0aa5710df5 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_api_create.py @@ -18,6 +18,7 @@ from models_library.workspaces import UserWorkspaceWithAccessRights from pydantic import TypeAdapter from servicelib.long_running_tasks.models import TaskProgress +from servicelib.long_running_tasks.task import TaskRegistry from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON from servicelib.redis import with_project_locked from servicelib.rest_constants import RESPONSE_MODEL_POLICY @@ -249,7 +250,7 @@ async def _compose_project_data( async def create_project( # pylint: disable=too-many-arguments,too-many-branches,too-many-statements # noqa: C901, PLR0913 - task_progress: TaskProgress, + progress: TaskProgress, *, request: web.Request, new_project_was_hidden_before_data_was_copied: bool, @@ -299,7 +300,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche copy_file_coro = None project_nodes = None try: - task_progress.update(message="creating new study...") + progress.update(message="creating new study...") workspace_id = None folder_id = None @@ -337,7 +338,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche src_project_uuid=from_study, as_template=as_template, deep_copy=copy_data, - task_progress=task_progress, + task_progress=progress, ) if project_node_coro: project_nodes = await project_node_coro @@ -387,7 +388,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche parent_project_uuid=parent_project_uuid, parent_node_id=parent_node_id, ) - task_progress.update() + progress.update() # 3.2 move project to proper folder if folder_id: @@ -415,7 +416,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche await dynamic_scheduler_service.update_projects_networks( request.app, project_id=ProjectID(new_project["uuid"]) ) - task_progress.update() + progress.update() # This is a new project and every new graph needs to be reflected in the pipeline tables await director_v2_service.create_or_update_pipeline( @@ -436,7 +437,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche is_template=as_template, app=request.app, ) - task_progress.update() + progress.update() # Adds permalink await update_or_pop_permalink_in_project(request, new_project) @@ -518,3 +519,6 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche simcore_user_agent=simcore_user_agent, ) raise + + +TaskRegistry.register(create_project) From a3a35b59fa40baa980c12ab4b18e7603161651b0 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 11:06:44 +0200 Subject: [PATCH 13/33] wired task_start via common interface --- .../aiohttp/long_running_tasks/_server.py | 4 +- .../http_endpoint_responses.py | 61 ++++++++++++----- .../src/servicelib/long_running_tasks/task.py | 33 ++-------- .../test_long_running_tasks.py | 4 +- ...test_long_running_tasks_context_manager.py | 9 ++- .../test_long_running_tasks_task.py | 65 +++++++++++++------ .../api/routes/dynamic_scheduler.py | 17 +++-- .../api/rest/containers_long_running_tasks.py | 28 +++++--- 8 files changed, 136 insertions(+), 85 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py index a4b77c7d4be..7c9a23421c3 100644 --- a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py +++ b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py @@ -9,6 +9,7 @@ from pydantic import AnyHttpUrl, TypeAdapter from ...aiohttp import status +from ...long_running_tasks import http_endpoint_responses from ...long_running_tasks.constants import ( DEFAULT_STALE_TASK_CHECK_INTERVAL, DEFAULT_STALE_TASK_DETECT_TIMEOUT, @@ -55,7 +56,8 @@ async def start_long_running_task( task_name = _create_task_name_from_request(request_) task_id = None try: - task_id = long_running_manager.tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, registerd_task_name, fire_and_forget=fire_and_forget, task_context=task_context, diff --git a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py b/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py index 056ca5a3bab..4f67861c7c2 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py +++ b/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py @@ -1,7 +1,7 @@ from typing import Any from .models import TaskBase, TaskId, TaskStatus -from .task import TaskContext, TasksManager, TrackedTask +from .task import RegisteredTaskName, TaskContext, TasksManager, TrackedTask def list_tasks( @@ -16,6 +16,7 @@ def list_tasks( def get_task_status( tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId ) -> TaskStatus: + """returns the status of a task""" return tasks_manager.get_task_status( task_id=task_id, with_task_context=task_context ) @@ -24,6 +25,7 @@ def get_task_status( async def get_task_result( tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId ) -> Any: + """retruns the result of a task, which is directly whatever the remove hanlder returned""" try: return tasks_manager.get_task_result( task_id=task_id, with_task_context=task_context @@ -38,23 +40,50 @@ async def get_task_result( async def remove_task( tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId ) -> None: + """removes / cancels a task""" await tasks_manager.remove_task(task_id, with_task_context=task_context) -# TODO reroute start via this with registration of hanlders +async def start_task( + tasks_manager: TasksManager, + registered_task_name: RegisteredTaskName, + *, + unique: bool = False, + task_context: TaskContext | None = None, + task_name: str | None = None, + fire_and_forget: bool = False, + **task_kwargs: Any, +) -> TaskId: + """ + Creates a background task from an async function. -# TODO: to support this we need handler registration capability in order to figure out if they can be started -# this should be done via the final TasksManager -# - also + An asyncio task will be created out of it by injecting a `TaskProgress` as the first + positional argument and adding all `handler_kwargs` as named parameters. -# TODO: this should be used to start everywhere not the client's start_task actually, -# this way we completly isolate everything and are allowed to call it form everywhere -# async def start_task( -# tasks_manager: TasksManager, -# task: TaskContext | None, -# task_context: TaskContext | None = None, -# **task_kwargs: Any, -# ) -> TaskId: -# return tasks_manager.start_task( -# task=task, task_context=task_context, **task_kwargs -# ) + NOTE: the progress is automatically bounded between 0 and 1 + NOTE: the `task` name must be unique in the module, otherwise when using + the unique parameter is True, it will not be able to distinguish between + them. + + Args: + tasks_manager (TasksManager): the tasks manager + task (TaskProtocol): the tasks to be run in the background + unique (bool, optional): If True, then only one such named task may be run. Defaults to False. + task_context (Optional[TaskContext], optional): a task context storage can be retrieved during the task lifetime. Defaults to None. + task_name (Optional[str], optional): optional task name. Defaults to None. + fire_and_forget: if True, then the task will not be cancelled if the status is never called + + Raises: + TaskAlreadyRunningError: if unique is True, will raise if more than 1 such named task is started + + Returns: + TaskId: the task unique identifier + """ + return tasks_manager.start_task( + registered_task_name, + unique=unique, + task_context=task_context, + task_name=task_name, + fire_and_forget=fire_and_forget, + **task_kwargs, + ) diff --git a/packages/service-library/src/servicelib/long_running_tasks/task.py b/packages/service-library/src/servicelib/long_running_tasks/task.py index 5bd6bbe0ca5..548ad6c0dc7 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/task.py +++ b/packages/service-library/src/servicelib/long_running_tasks/task.py @@ -335,37 +335,12 @@ def start_task( self, registered_task_name: RegisteredTaskName, *, - unique: bool = False, - task_context: TaskContext | None = None, - task_name: str | None = None, - fire_and_forget: bool = False, + unique: bool, + task_context: TaskContext | None, + task_name: str | None, + fire_and_forget: bool, **task_kwargs: Any, ) -> TaskId: - """ - Creates a background task from an async function. - - An asyncio task will be created out of it by injecting a `TaskProgress` as the first - positional argument and adding all `handler_kwargs` as named parameters. - - NOTE: the progress is automatically bounded between 0 and 1 - NOTE: the `task` name must be unique in the module, otherwise when using - the unique parameter is True, it will not be able to distinguish between - them. - - Args: - tasks_manager (TasksManager): the tasks manager - task (TaskProtocol): the tasks to be run in the background - unique (bool, optional): If True, then only one such named task may be run. Defaults to False. - task_context (Optional[TaskContext], optional): a task context storage can be retrieved during the task lifetime. Defaults to None. - task_name (Optional[str], optional): optional task name. Defaults to None. - fire_and_forget: if True, then the task will not be cancelled if the status is never called - - Raises: - TaskAlreadyRunningError: if unique is True, will raise if more than 1 such named task is started - - Returns: - TaskId: the task unique identifier - """ if registered_task_name not in TaskRegistry.REGISTERED_TASKS: raise TaskNotRegisteredError(task_name=registered_task_name) diff --git a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py index 428d1d3ab8c..77bb27a72d7 100644 --- a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py +++ b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py @@ -25,6 +25,7 @@ get_long_running_manager, ) from servicelib.fastapi.long_running_tasks.server import setup as setup_server +from servicelib.long_running_tasks import http_endpoint_responses from servicelib.long_running_tasks.models import ( TaskGet, TaskId, @@ -78,7 +79,8 @@ async def create_string_list_task( *, fail: bool = False, ) -> TaskId: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, _string_list_task.__name__, num_strings=num_strings, sleep_time=sleep_time, diff --git a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py index e0febe73707..4a5d0e02a3b 100644 --- a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py +++ b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py @@ -16,6 +16,7 @@ from servicelib.fastapi.long_running_tasks.client import setup as setup_client from servicelib.fastapi.long_running_tasks.server import get_long_running_manager from servicelib.fastapi.long_running_tasks.server import setup as setup_server +from servicelib.long_running_tasks import http_endpoint_responses from servicelib.long_running_tasks.errors import ( TaskClientTimeoutError, ) @@ -68,7 +69,9 @@ async def create_task_user_defined_route( FastAPILongRunningManager, Depends(get_long_running_manager) ], ) -> TaskId: - return long_running_manager.tasks_manager.start_task(a_test_task.__name__) + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, a_test_task.__name__ + ) @router.get("/api/failing", status_code=status.HTTP_200_OK) async def create_task_which_fails( @@ -76,8 +79,8 @@ async def create_task_which_fails( FastAPILongRunningManager, Depends(get_long_running_manager) ], ) -> TaskId: - return long_running_manager.tasks_manager.start_task( - a_failing_test_task.__name__ + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, a_failing_test_task.__name__ ) return router diff --git a/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py b/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py index 2e32bfde908..261e982fdc0 100644 --- a/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py +++ b/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py @@ -12,6 +12,7 @@ import pytest from faker import Faker +from servicelib.long_running_tasks import http_endpoint_responses from servicelib.long_running_tasks.errors import ( TaskAlreadyRunningError, TaskCancelledError, @@ -84,7 +85,8 @@ async def tasks_manager() -> AsyncIterator[TasksManager]: async def test_task_is_auto_removed( tasks_manager: TasksManager, check_task_presence_before: bool ): - task_id = tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10 * TEST_CHECK_STALE_INTERVAL_S, @@ -110,7 +112,8 @@ async def test_task_is_auto_removed( async def test_checked_task_is_not_auto_removed(tasks_manager: TasksManager): - task_id = tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=5 * TEST_CHECK_STALE_INTERVAL_S, @@ -124,7 +127,8 @@ async def test_checked_task_is_not_auto_removed(tasks_manager: TasksManager): async def test_fire_and_forget_task_is_not_auto_removed(tasks_manager: TasksManager): - task_id = tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=5 * TEST_CHECK_STALE_INTERVAL_S, @@ -142,7 +146,8 @@ async def test_fire_and_forget_task_is_not_auto_removed(tasks_manager: TasksMana async def test_get_result_of_unfinished_task_raises(tasks_manager: TasksManager): - task_id = tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=5 * TEST_CHECK_STALE_INTERVAL_S, @@ -158,11 +163,15 @@ async def unique_task(progress: TaskProgress): TaskRegistry.register(unique_task) - tasks_manager.start_task(unique_task.__name__, unique=True) + await http_endpoint_responses.start_task( + tasks_manager, unique_task.__name__, unique=True + ) # ensure unique running task regardless of how many times it gets started with pytest.raises(TaskAlreadyRunningError) as exec_info: - tasks_manager.start_task(unique_task.__name__, unique=True) + await http_endpoint_responses.start_task( + tasks_manager, unique_task.__name__, unique=True + ) assert "must be unique, found: " in f"{exec_info.value}" TaskRegistry.unregister(unique_task) @@ -175,7 +184,9 @@ async def not_unique_task(progress: TaskProgress): TaskRegistry.register(not_unique_task) for _ in range(5): - tasks_manager.start_task(not_unique_task.__name__) + await http_endpoint_responses.start_task( + tasks_manager, not_unique_task.__name__ + ) TaskRegistry.unregister(not_unique_task) @@ -188,7 +199,8 @@ def test_get_task_id(tasks_manager: TasksManager, faker: Faker, is_unique: bool) async def test_get_status(tasks_manager: TasksManager): - task_id = tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, @@ -208,7 +220,9 @@ async def test_get_status_missing(tasks_manager: TasksManager): async def test_get_result(tasks_manager: TasksManager): - task_id = tasks_manager.start_task(fast_background_task.__name__) + task_id = await http_endpoint_responses.start_task( + tasks_manager, fast_background_task.__name__ + ) await asyncio.sleep(0.1) result = tasks_manager.get_task_result(task_id, with_task_context=None) assert result == 42 @@ -221,7 +235,9 @@ async def test_get_result_missing(tasks_manager: TasksManager): async def test_get_result_finished_with_error(tasks_manager: TasksManager): - task_id = tasks_manager.start_task(failing_background_task.__name__) + task_id = await http_endpoint_responses.start_task( + tasks_manager, failing_background_task.__name__ + ) # wait for result async for attempt in AsyncRetrying(**_RETRY_PARAMS): with attempt: @@ -234,7 +250,8 @@ async def test_get_result_finished_with_error(tasks_manager: TasksManager): async def test_get_result_task_was_cancelled_multiple_times( tasks_manager: TasksManager, ): - task_id = tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, @@ -249,7 +266,8 @@ async def test_get_result_task_was_cancelled_multiple_times( async def test_remove_task(tasks_manager: TasksManager): - task_id = tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, @@ -264,7 +282,8 @@ async def test_remove_task(tasks_manager: TasksManager): async def test_remove_task_with_task_context(tasks_manager: TasksManager): TASK_CONTEXT = {"some_context": "some_value"} - task_id = tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, @@ -296,7 +315,8 @@ async def test_remove_unknown_task(tasks_manager: TasksManager): async def test_cancel_task_with_task_context(tasks_manager: TasksManager): TASK_CONTEXT = {"some_context": "some_value"} - task_id = tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, @@ -322,7 +342,8 @@ async def test_list_tasks(tasks_manager: TasksManager): task_ids = [] for _ in range(NUM_TASKS): task_ids.append( # noqa: PERF401 - tasks_manager.start_task( + await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, @@ -337,18 +358,21 @@ async def test_list_tasks(tasks_manager: TasksManager): async def test_list_tasks_filtering(tasks_manager: TasksManager): - tasks_manager.start_task( + await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, ) - tasks_manager.start_task( + await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, task_context={"user_id": 213}, ) - tasks_manager.start_task( + await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, @@ -376,7 +400,8 @@ async def test_list_tasks_filtering(tasks_manager: TasksManager): async def test_define_task_name(tasks_manager: TasksManager, faker: Faker): task_name = faker.name() - task_id = tasks_manager.start_task( + task_id = await http_endpoint_responses.start_task( + tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, @@ -387,4 +412,4 @@ async def test_define_task_name(tasks_manager: TasksManager, faker: Faker): async def test_start_not_registered_task(tasks_manager: TasksManager): with pytest.raises(TaskNotRegisteredError): - tasks_manager.start_task("not_registered_task") + await http_endpoint_responses.start_task(tasks_manager, "not_registered_task") diff --git a/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py b/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py index 7ccc50376d3..36ff61838d4 100644 --- a/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py +++ b/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py @@ -6,6 +6,7 @@ from pydantic import BaseModel, PositiveInt from servicelib.fastapi.long_running_tasks._manager import FastAPILongRunningManager from servicelib.fastapi.long_running_tasks.server import get_long_running_manager +from servicelib.long_running_tasks import http_endpoint_responses from servicelib.long_running_tasks.errors import TaskAlreadyRunningError from servicelib.long_running_tasks.models import ( ProgressMessage, @@ -114,7 +115,8 @@ async def _progress_callback( TaskRegistry.register(_task_remove_service_containers) try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, _task_remove_service_containers.__name__, unique=True, node_uuid=node_uuid, @@ -178,7 +180,8 @@ async def _progress_callback( TaskRegistry.register(_task_save_service_state) try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, _task_save_service_state.__name__, unique=True, node_uuid=node_uuid, @@ -224,8 +227,9 @@ async def _progress_callback( TaskRegistry.register(_task_push_service_outputs) try: - return long_running_manager.tasks_manager.start_task( - task=_task_push_service_outputs, # type: ignore[arg-type] + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, + _task_push_service_outputs.__name__, unique=True, node_uuid=node_uuid, ) @@ -265,8 +269,9 @@ async def _task_cleanup_service_docker_resources( TaskRegistry.register(_task_cleanup_service_docker_resources) try: - return long_running_manager.tasks_manager.start_task( - task=_task_cleanup_service_docker_resources, # type: ignore[arg-type] + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, + _task_cleanup_service_docker_resources.__name__, unique=True, node_uuid=node_uuid, ) diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py index 14d6f8cdbbd..568dd49b97d 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py @@ -5,6 +5,7 @@ from servicelib.fastapi.long_running_tasks._manager import FastAPILongRunningManager from servicelib.fastapi.long_running_tasks.server import get_long_running_manager from servicelib.fastapi.requests_decorators import cancel_on_disconnect +from servicelib.long_running_tasks import http_endpoint_responses from servicelib.long_running_tasks.errors import TaskAlreadyRunningError from servicelib.long_running_tasks.models import TaskId @@ -57,7 +58,8 @@ async def pull_user_servcices_docker_images( assert request # nosec try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, task_pull_user_servcices_docker_images.__name__, unique=True, app=app, @@ -97,7 +99,8 @@ async def create_service_containers_task( # pylint: disable=too-many-arguments assert request # nosec try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, task_create_service_containers.__name__, unique=True, settings=settings, @@ -130,7 +133,8 @@ async def runs_docker_compose_down_task( assert request # nosec try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, task_runs_docker_compose_down.__name__, unique=True, app=app, @@ -161,7 +165,8 @@ async def state_restore_task( assert request # nosec try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, task_restore_state.__name__, unique=True, settings=settings, @@ -191,7 +196,8 @@ async def state_save_task( assert request # nosec try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, task_save_state.__name__, unique=True, settings=settings, @@ -223,7 +229,8 @@ async def ports_inputs_pull_task( assert request # nosec try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, task_ports_inputs_pull.__name__, unique=True, port_keys=port_keys, @@ -255,7 +262,8 @@ async def ports_outputs_pull_task( assert request # nosec try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, task_ports_outputs_pull.__name__, unique=True, port_keys=port_keys, @@ -284,7 +292,8 @@ async def ports_outputs_push_task( assert request # nosec try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, task_ports_outputs_push.__name__, unique=True, outputs_manager=outputs_manager, @@ -313,7 +322,8 @@ async def containers_restart_task( assert request # nosec try: - return long_running_manager.tasks_manager.start_task( + return await http_endpoint_responses.start_task( + long_running_manager.tasks_manager, task_containers_restart.__name__, unique=True, app=app, From 216c93aa294ae14a50784cfd65f47999b28329b4 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 11:07:32 +0200 Subject: [PATCH 14/33] refactor --- .../http_endpoint_responses.py | 80 +++++++++---------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py b/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py index 4f67861c7c2..771dd5eec22 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py +++ b/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py @@ -4,46 +4,6 @@ from .task import RegisteredTaskName, TaskContext, TasksManager, TrackedTask -def list_tasks( - tasks_manager: TasksManager, task_context: TaskContext | None -) -> list[TaskBase]: - tracked_tasks: list[TrackedTask] = tasks_manager.list_tasks( - with_task_context=task_context - ) - return [TaskBase(task_id=t.task_id) for t in tracked_tasks] - - -def get_task_status( - tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId -) -> TaskStatus: - """returns the status of a task""" - return tasks_manager.get_task_status( - task_id=task_id, with_task_context=task_context - ) - - -async def get_task_result( - tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId -) -> Any: - """retruns the result of a task, which is directly whatever the remove hanlder returned""" - try: - return tasks_manager.get_task_result( - task_id=task_id, with_task_context=task_context - ) - finally: - # the task is always removed even if an error occurs - await tasks_manager.remove_task( - task_id, with_task_context=task_context, reraise_errors=False - ) - - -async def remove_task( - tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId -) -> None: - """removes / cancels a task""" - await tasks_manager.remove_task(task_id, with_task_context=task_context) - - async def start_task( tasks_manager: TasksManager, registered_task_name: RegisteredTaskName, @@ -87,3 +47,43 @@ async def start_task( fire_and_forget=fire_and_forget, **task_kwargs, ) + + +def list_tasks( + tasks_manager: TasksManager, task_context: TaskContext | None +) -> list[TaskBase]: + tracked_tasks: list[TrackedTask] = tasks_manager.list_tasks( + with_task_context=task_context + ) + return [TaskBase(task_id=t.task_id) for t in tracked_tasks] + + +def get_task_status( + tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId +) -> TaskStatus: + """returns the status of a task""" + return tasks_manager.get_task_status( + task_id=task_id, with_task_context=task_context + ) + + +async def get_task_result( + tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId +) -> Any: + """retruns the result of a task, which is directly whatever the remove hanlder returned""" + try: + return tasks_manager.get_task_result( + task_id=task_id, with_task_context=task_context + ) + finally: + # the task is always removed even if an error occurs + await tasks_manager.remove_task( + task_id, with_task_context=task_context, reraise_errors=False + ) + + +async def remove_task( + tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId +) -> None: + """removes / cancels a task""" + await tasks_manager.remove_task(task_id, with_task_context=task_context) From 0ed5b0702e56454ede60adf469ab9887526886fc Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 11:09:18 +0200 Subject: [PATCH 15/33] renamed module --- .../aiohttp/long_running_tasks/_routes.py | 10 ++-- .../aiohttp/long_running_tasks/_server.py | 4 +- .../fastapi/long_running_tasks/_routes.py | 10 ++-- ...{http_endpoint_responses.py => lrt_api.py} | 0 .../test_long_running_tasks.py | 4 +- ...test_long_running_tasks_context_manager.py | 6 +-- .../test_long_running_tasks_task.py | 52 ++++++++----------- .../api/routes/dynamic_scheduler.py | 10 ++-- .../api/rest/containers_long_running_tasks.py | 20 +++---- .../simcore_service_webserver/tasks/_rest.py | 4 +- 10 files changed, 55 insertions(+), 65 deletions(-) rename packages/service-library/src/servicelib/long_running_tasks/{http_endpoint_responses.py => lrt_api.py} (100%) diff --git a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_routes.py b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_routes.py index 11f7dcd4d7e..ec036dcb1c5 100644 --- a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_routes.py +++ b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_routes.py @@ -4,7 +4,7 @@ from pydantic import BaseModel from servicelib.aiohttp import status -from ...long_running_tasks import http_endpoint_responses +from ...long_running_tasks import lrt_api from ...long_running_tasks.models import TaskGet, TaskId, TaskStatus from ..requests_validation import parse_request_path_parameters_as from ..rest_responses import create_data_response @@ -28,7 +28,7 @@ async def list_tasks(request: web.Request) -> web.Response: result_href=f"{request.app.router['get_task_result'].url_for(task_id=t.task_id)}", abort_href=f"{request.app.router['cancel_and_delete_task'].url_for(task_id=t.task_id)}", ) - for t in http_endpoint_responses.list_tasks( + for t in lrt_api.list_tasks( long_running_manager.tasks_manager, long_running_manager.get_task_context(request), ) @@ -41,7 +41,7 @@ async def get_task_status(request: web.Request) -> web.Response: path_params = parse_request_path_parameters_as(_PathParam, request) long_running_manager = get_long_running_manager(request.app) - task_status: TaskStatus = http_endpoint_responses.get_task_status( + task_status: TaskStatus = lrt_api.get_task_status( long_running_manager.tasks_manager, long_running_manager.get_task_context(request), path_params.task_id, @@ -55,7 +55,7 @@ async def get_task_result(request: web.Request) -> web.Response | Any: long_running_manager = get_long_running_manager(request.app) # NOTE: this might raise an exception that will be catched by the _error_handlers - return await http_endpoint_responses.get_task_result( + return await lrt_api.get_task_result( long_running_manager.tasks_manager, long_running_manager.get_task_context(request), path_params.task_id, @@ -67,7 +67,7 @@ async def cancel_and_delete_task(request: web.Request) -> web.Response: path_params = parse_request_path_parameters_as(_PathParam, request) long_running_manager = get_long_running_manager(request.app) - await http_endpoint_responses.remove_task( + await lrt_api.remove_task( long_running_manager.tasks_manager, long_running_manager.get_task_context(request), path_params.task_id, diff --git a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py index 7c9a23421c3..e8b2efad1dc 100644 --- a/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py +++ b/packages/service-library/src/servicelib/aiohttp/long_running_tasks/_server.py @@ -9,7 +9,7 @@ from pydantic import AnyHttpUrl, TypeAdapter from ...aiohttp import status -from ...long_running_tasks import http_endpoint_responses +from ...long_running_tasks import lrt_api from ...long_running_tasks.constants import ( DEFAULT_STALE_TASK_CHECK_INTERVAL, DEFAULT_STALE_TASK_DETECT_TIMEOUT, @@ -56,7 +56,7 @@ async def start_long_running_task( task_name = _create_task_name_from_request(request_) task_id = None try: - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( long_running_manager.tasks_manager, registerd_task_name, fire_and_forget=fire_and_forget, diff --git a/packages/service-library/src/servicelib/fastapi/long_running_tasks/_routes.py b/packages/service-library/src/servicelib/fastapi/long_running_tasks/_routes.py index ff82a9ca956..5f8a5e0a633 100644 --- a/packages/service-library/src/servicelib/fastapi/long_running_tasks/_routes.py +++ b/packages/service-library/src/servicelib/fastapi/long_running_tasks/_routes.py @@ -2,7 +2,7 @@ from fastapi import APIRouter, Depends, Request, status -from ...long_running_tasks import http_endpoint_responses +from ...long_running_tasks import lrt_api from ...long_running_tasks.models import TaskGet, TaskId, TaskResult, TaskStatus from ..requests_decorators import cancel_on_disconnect from ._dependencies import get_long_running_manager @@ -29,7 +29,7 @@ async def list_tasks( request.url_for("cancel_and_delete_task", task_id=t.task_id) ), ) - for t in http_endpoint_responses.list_tasks( + for t in lrt_api.list_tasks( long_running_manager.tasks_manager, task_context=None ) ] @@ -51,7 +51,7 @@ async def get_task_status( ], ) -> TaskStatus: assert request # nosec - return http_endpoint_responses.get_task_status( + return lrt_api.get_task_status( long_running_manager.tasks_manager, task_context=None, task_id=task_id ) @@ -74,7 +74,7 @@ async def get_task_result( ], ) -> TaskResult | Any: assert request # nosec - return await http_endpoint_responses.get_task_result( + return await lrt_api.get_task_result( long_running_manager.tasks_manager, task_context=None, task_id=task_id ) @@ -97,6 +97,6 @@ async def cancel_and_delete_task( ], ) -> None: assert request # nosec - await http_endpoint_responses.remove_task( + await lrt_api.remove_task( long_running_manager.tasks_manager, task_context=None, task_id=task_id ) diff --git a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py b/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py similarity index 100% rename from packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py rename to packages/service-library/src/servicelib/long_running_tasks/lrt_api.py diff --git a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py index 77bb27a72d7..5ebbdb744e0 100644 --- a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py +++ b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks.py @@ -25,7 +25,7 @@ get_long_running_manager, ) from servicelib.fastapi.long_running_tasks.server import setup as setup_server -from servicelib.long_running_tasks import http_endpoint_responses +from servicelib.long_running_tasks import lrt_api from servicelib.long_running_tasks.models import ( TaskGet, TaskId, @@ -79,7 +79,7 @@ async def create_string_list_task( *, fail: bool = False, ) -> TaskId: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, _string_list_task.__name__, num_strings=num_strings, diff --git a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py index 4a5d0e02a3b..9f497f63baa 100644 --- a/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py +++ b/packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py @@ -16,7 +16,7 @@ from servicelib.fastapi.long_running_tasks.client import setup as setup_client from servicelib.fastapi.long_running_tasks.server import get_long_running_manager from servicelib.fastapi.long_running_tasks.server import setup as setup_server -from servicelib.long_running_tasks import http_endpoint_responses +from servicelib.long_running_tasks import lrt_api from servicelib.long_running_tasks.errors import ( TaskClientTimeoutError, ) @@ -69,7 +69,7 @@ async def create_task_user_defined_route( FastAPILongRunningManager, Depends(get_long_running_manager) ], ) -> TaskId: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, a_test_task.__name__ ) @@ -79,7 +79,7 @@ async def create_task_which_fails( FastAPILongRunningManager, Depends(get_long_running_manager) ], ) -> TaskId: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, a_failing_test_task.__name__ ) diff --git a/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py b/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py index 261e982fdc0..566be002808 100644 --- a/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py +++ b/packages/service-library/tests/long_running_tasks/test_long_running_tasks_task.py @@ -12,7 +12,7 @@ import pytest from faker import Faker -from servicelib.long_running_tasks import http_endpoint_responses +from servicelib.long_running_tasks import lrt_api from servicelib.long_running_tasks.errors import ( TaskAlreadyRunningError, TaskCancelledError, @@ -85,7 +85,7 @@ async def tasks_manager() -> AsyncIterator[TasksManager]: async def test_task_is_auto_removed( tasks_manager: TasksManager, check_task_presence_before: bool ): - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -112,7 +112,7 @@ async def test_task_is_auto_removed( async def test_checked_task_is_not_auto_removed(tasks_manager: TasksManager): - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -127,7 +127,7 @@ async def test_checked_task_is_not_auto_removed(tasks_manager: TasksManager): async def test_fire_and_forget_task_is_not_auto_removed(tasks_manager: TasksManager): - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -146,7 +146,7 @@ async def test_fire_and_forget_task_is_not_auto_removed(tasks_manager: TasksMana async def test_get_result_of_unfinished_task_raises(tasks_manager: TasksManager): - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -163,15 +163,11 @@ async def unique_task(progress: TaskProgress): TaskRegistry.register(unique_task) - await http_endpoint_responses.start_task( - tasks_manager, unique_task.__name__, unique=True - ) + await lrt_api.start_task(tasks_manager, unique_task.__name__, unique=True) # ensure unique running task regardless of how many times it gets started with pytest.raises(TaskAlreadyRunningError) as exec_info: - await http_endpoint_responses.start_task( - tasks_manager, unique_task.__name__, unique=True - ) + await lrt_api.start_task(tasks_manager, unique_task.__name__, unique=True) assert "must be unique, found: " in f"{exec_info.value}" TaskRegistry.unregister(unique_task) @@ -184,9 +180,7 @@ async def not_unique_task(progress: TaskProgress): TaskRegistry.register(not_unique_task) for _ in range(5): - await http_endpoint_responses.start_task( - tasks_manager, not_unique_task.__name__ - ) + await lrt_api.start_task(tasks_manager, not_unique_task.__name__) TaskRegistry.unregister(not_unique_task) @@ -199,7 +193,7 @@ def test_get_task_id(tasks_manager: TasksManager, faker: Faker, is_unique: bool) async def test_get_status(tasks_manager: TasksManager): - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -220,9 +214,7 @@ async def test_get_status_missing(tasks_manager: TasksManager): async def test_get_result(tasks_manager: TasksManager): - task_id = await http_endpoint_responses.start_task( - tasks_manager, fast_background_task.__name__ - ) + task_id = await lrt_api.start_task(tasks_manager, fast_background_task.__name__) await asyncio.sleep(0.1) result = tasks_manager.get_task_result(task_id, with_task_context=None) assert result == 42 @@ -235,9 +227,7 @@ async def test_get_result_missing(tasks_manager: TasksManager): async def test_get_result_finished_with_error(tasks_manager: TasksManager): - task_id = await http_endpoint_responses.start_task( - tasks_manager, failing_background_task.__name__ - ) + task_id = await lrt_api.start_task(tasks_manager, failing_background_task.__name__) # wait for result async for attempt in AsyncRetrying(**_RETRY_PARAMS): with attempt: @@ -250,7 +240,7 @@ async def test_get_result_finished_with_error(tasks_manager: TasksManager): async def test_get_result_task_was_cancelled_multiple_times( tasks_manager: TasksManager, ): - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -266,7 +256,7 @@ async def test_get_result_task_was_cancelled_multiple_times( async def test_remove_task(tasks_manager: TasksManager): - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -282,7 +272,7 @@ async def test_remove_task(tasks_manager: TasksManager): async def test_remove_task_with_task_context(tasks_manager: TasksManager): TASK_CONTEXT = {"some_context": "some_value"} - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -315,7 +305,7 @@ async def test_remove_unknown_task(tasks_manager: TasksManager): async def test_cancel_task_with_task_context(tasks_manager: TasksManager): TASK_CONTEXT = {"some_context": "some_value"} - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -342,7 +332,7 @@ async def test_list_tasks(tasks_manager: TasksManager): task_ids = [] for _ in range(NUM_TASKS): task_ids.append( # noqa: PERF401 - await http_endpoint_responses.start_task( + await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -358,20 +348,20 @@ async def test_list_tasks(tasks_manager: TasksManager): async def test_list_tasks_filtering(tasks_manager: TasksManager): - await http_endpoint_responses.start_task( + await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, ) - await http_endpoint_responses.start_task( + await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, total_sleep=10, task_context={"user_id": 213}, ) - await http_endpoint_responses.start_task( + await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -400,7 +390,7 @@ async def test_list_tasks_filtering(tasks_manager: TasksManager): async def test_define_task_name(tasks_manager: TasksManager, faker: Faker): task_name = faker.name() - task_id = await http_endpoint_responses.start_task( + task_id = await lrt_api.start_task( tasks_manager, a_background_task.__name__, raise_when_finished=False, @@ -412,4 +402,4 @@ async def test_define_task_name(tasks_manager: TasksManager, faker: Faker): async def test_start_not_registered_task(tasks_manager: TasksManager): with pytest.raises(TaskNotRegisteredError): - await http_endpoint_responses.start_task(tasks_manager, "not_registered_task") + await lrt_api.start_task(tasks_manager, "not_registered_task") diff --git a/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py b/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py index 36ff61838d4..bbec37d8d45 100644 --- a/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py +++ b/services/director-v2/src/simcore_service_director_v2/api/routes/dynamic_scheduler.py @@ -6,7 +6,7 @@ from pydantic import BaseModel, PositiveInt from servicelib.fastapi.long_running_tasks._manager import FastAPILongRunningManager from servicelib.fastapi.long_running_tasks.server import get_long_running_manager -from servicelib.long_running_tasks import http_endpoint_responses +from servicelib.long_running_tasks import lrt_api from servicelib.long_running_tasks.errors import TaskAlreadyRunningError from servicelib.long_running_tasks.models import ( ProgressMessage, @@ -115,7 +115,7 @@ async def _progress_callback( TaskRegistry.register(_task_remove_service_containers) try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, _task_remove_service_containers.__name__, unique=True, @@ -180,7 +180,7 @@ async def _progress_callback( TaskRegistry.register(_task_save_service_state) try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, _task_save_service_state.__name__, unique=True, @@ -227,7 +227,7 @@ async def _progress_callback( TaskRegistry.register(_task_push_service_outputs) try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, _task_push_service_outputs.__name__, unique=True, @@ -269,7 +269,7 @@ async def _task_cleanup_service_docker_resources( TaskRegistry.register(_task_cleanup_service_docker_resources) try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, _task_cleanup_service_docker_resources.__name__, unique=True, diff --git a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py index 568dd49b97d..7e9fdb3d0b8 100644 --- a/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py +++ b/services/dynamic-sidecar/src/simcore_service_dynamic_sidecar/api/rest/containers_long_running_tasks.py @@ -5,7 +5,7 @@ from servicelib.fastapi.long_running_tasks._manager import FastAPILongRunningManager from servicelib.fastapi.long_running_tasks.server import get_long_running_manager from servicelib.fastapi.requests_decorators import cancel_on_disconnect -from servicelib.long_running_tasks import http_endpoint_responses +from servicelib.long_running_tasks import lrt_api from servicelib.long_running_tasks.errors import TaskAlreadyRunningError from servicelib.long_running_tasks.models import TaskId @@ -58,7 +58,7 @@ async def pull_user_servcices_docker_images( assert request # nosec try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, task_pull_user_servcices_docker_images.__name__, unique=True, @@ -99,7 +99,7 @@ async def create_service_containers_task( # pylint: disable=too-many-arguments assert request # nosec try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, task_create_service_containers.__name__, unique=True, @@ -133,7 +133,7 @@ async def runs_docker_compose_down_task( assert request # nosec try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, task_runs_docker_compose_down.__name__, unique=True, @@ -165,7 +165,7 @@ async def state_restore_task( assert request # nosec try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, task_restore_state.__name__, unique=True, @@ -196,7 +196,7 @@ async def state_save_task( assert request # nosec try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, task_save_state.__name__, unique=True, @@ -229,7 +229,7 @@ async def ports_inputs_pull_task( assert request # nosec try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, task_ports_inputs_pull.__name__, unique=True, @@ -262,7 +262,7 @@ async def ports_outputs_pull_task( assert request # nosec try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, task_ports_outputs_pull.__name__, unique=True, @@ -292,7 +292,7 @@ async def ports_outputs_push_task( assert request # nosec try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, task_ports_outputs_push.__name__, unique=True, @@ -322,7 +322,7 @@ async def containers_restart_task( assert request # nosec try: - return await http_endpoint_responses.start_task( + return await lrt_api.start_task( long_running_manager.tasks_manager, task_containers_restart.__name__, unique=True, diff --git a/services/web/server/src/simcore_service_webserver/tasks/_rest.py b/services/web/server/src/simcore_service_webserver/tasks/_rest.py index b45ff0a1260..c0795c8137c 100644 --- a/services/web/server/src/simcore_service_webserver/tasks/_rest.py +++ b/services/web/server/src/simcore_service_webserver/tasks/_rest.py @@ -28,7 +28,7 @@ parse_request_path_parameters_as, ) from servicelib.aiohttp.rest_responses import create_data_response -from servicelib.long_running_tasks import http_endpoint_responses +from servicelib.long_running_tasks import lrt_api from servicelib.rabbitmq.rpc_interfaces.async_jobs import async_jobs from .._meta import API_VTAG @@ -57,7 +57,7 @@ @webserver_request_context_decorator async def get_async_jobs(request: web.Request) -> web.Response: inprocess_long_running_manager = get_long_running_manager(request.app) - inprocess_tracked_tasks = http_endpoint_responses.list_tasks( + inprocess_tracked_tasks = lrt_api.list_tasks( inprocess_long_running_manager.tasks_manager, inprocess_long_running_manager.get_task_context(request), ) From 6c9cae3fe67b2eecb0789f1a91b5b46f6f7dbc0c Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 11:58:52 +0200 Subject: [PATCH 16/33] bring back task_name --- .../api_schemas_long_running_tasks/tasks.py | 19 ++++++++++++++++++- .../api/v0/openapi.yaml | 4 ++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py index aeaa710d4c4..80df6dcaf1e 100644 --- a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py +++ b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py @@ -1,7 +1,8 @@ +import urllib.parse from datetime import datetime from typing import Any -from pydantic import BaseModel +from pydantic import BaseModel, field_validator from .base import TaskId, TaskProgress @@ -20,6 +21,22 @@ class TaskResult(BaseModel): class TaskBase(BaseModel): task_id: TaskId + # NOTE: task name can always be extraced from the task_id + # since it'e encoded inside it + task_name: str = "" + + @field_validator("task_name", mode="before") + @classmethod + def populate_task_name(cls, _, info): + task_name = "" + task_id = info.data.get("task_id") + if task_id: + parts = task_id.split(".") + if len(parts) >= 1: + task_name = parts[1] + + return urllib.parse.unquote(task_name) + class TaskGet(TaskBase): status_href: str diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 7ec97ca2efb..73e8b2d4d7e 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -16716,6 +16716,9 @@ components: task_id: type: string title: Task Id + task_name: + type: string + title: Task Name status_href: type: string title: Status Href @@ -16728,6 +16731,7 @@ components: type: object required: - task_id + - task_name - status_href - result_href - abort_href From 69697c977cfac88805ca5de57960c717febccd04 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 13:00:45 +0200 Subject: [PATCH 17/33] fixed broken tests --- .../api_schemas_long_running_tasks/tasks.py | 9 ++++++--- .../unit/test_api_rest_containers_long_running_tasks.py | 5 +++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py index 80df6dcaf1e..81a99dbf20d 100644 --- a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py +++ b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py @@ -27,12 +27,15 @@ class TaskBase(BaseModel): @field_validator("task_name", mode="before") @classmethod - def populate_task_name(cls, _, info): - task_name = "" + def populate_task_name(cls, task_id, info): + task_name = task_id + + # attempt to solve the task name from the task_id + # if this is coming form a long_running_task task_id = info.data.get("task_id") if task_id: parts = task_id.split(".") - if len(parts) >= 1: + if len(parts) >= 2: task_name = parts[1] return urllib.parse.unquote(task_name) diff --git a/services/dynamic-sidecar/tests/unit/test_api_rest_containers_long_running_tasks.py b/services/dynamic-sidecar/tests/unit/test_api_rest_containers_long_running_tasks.py index 9218315945f..04c3f47e3b5 100644 --- a/services/dynamic-sidecar/tests/unit/test_api_rest_containers_long_running_tasks.py +++ b/services/dynamic-sidecar/tests/unit/test_api_rest_containers_long_running_tasks.py @@ -31,6 +31,7 @@ from servicelib.fastapi.long_running_tasks.client import Client, periodic_task_result from servicelib.fastapi.long_running_tasks.client import setup as client_setup from servicelib.long_running_tasks.models import TaskId +from servicelib.long_running_tasks.task import TaskRegistry from simcore_sdk.node_ports_common.exceptions import NodeNotFound from simcore_service_dynamic_sidecar._meta import API_VTAG from simcore_service_dynamic_sidecar.api.rest import containers_long_running_tasks @@ -75,6 +76,8 @@ def mock_tasks(mocker: MockerFixture) -> Iterator[None]: async def _just_log_task(*args, **kwargs) -> None: print(f"Called mocked function with {args}, {kwargs}") + TaskRegistry.register(_just_log_task) + # searching by name since all start with _task tasks_names = [ x[0] @@ -89,6 +92,8 @@ async def _just_log_task(*args, **kwargs) -> None: yield None + TaskRegistry.unregister(_just_log_task) + @asynccontextmanager async def auto_remove_task(client: Client, task_id: TaskId) -> AsyncIterator[None]: From 46b2ff3f4726c67183d0409bfbbb70af16b4121e Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 13:13:56 +0200 Subject: [PATCH 18/33] updated specs --- .../server/src/simcore_service_webserver/api/v0/openapi.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 73e8b2d4d7e..59d30492065 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -16719,6 +16719,7 @@ components: task_name: type: string title: Task Name + default: '' status_href: type: string title: Status Href @@ -16731,7 +16732,6 @@ components: type: object required: - task_id - - task_name - status_href - result_href - abort_href From 55bf41411ee55e53ea7c0cbce1839e5ad502d156 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 13:14:25 +0200 Subject: [PATCH 19/33] hid internals --- .../src/servicelib/long_running_tasks/lrt_api.py | 7 ++----- .../src/servicelib/long_running_tasks/task.py | 10 ++++++---- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py b/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py index 771dd5eec22..dbda9376aca 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py +++ b/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py @@ -1,7 +1,7 @@ from typing import Any from .models import TaskBase, TaskId, TaskStatus -from .task import RegisteredTaskName, TaskContext, TasksManager, TrackedTask +from .task import RegisteredTaskName, TaskContext, TasksManager async def start_task( @@ -52,10 +52,7 @@ async def start_task( def list_tasks( tasks_manager: TasksManager, task_context: TaskContext | None ) -> list[TaskBase]: - tracked_tasks: list[TrackedTask] = tasks_manager.list_tasks( - with_task_context=task_context - ) - return [TaskBase(task_id=t.task_id) for t in tracked_tasks] + return tasks_manager.list_tasks(with_task_context=task_context) def get_task_status( diff --git a/packages/service-library/src/servicelib/long_running_tasks/task.py b/packages/service-library/src/servicelib/long_running_tasks/task.py index 548ad6c0dc7..d15622dc67a 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/task.py +++ b/packages/service-library/src/servicelib/long_running_tasks/task.py @@ -23,7 +23,7 @@ TaskNotFoundError, TaskNotRegisteredError, ) -from .models import TaskId, TaskStatus, TrackedTask +from .models import TaskBase, TaskId, TaskStatus, TrackedTask _logger = logging.getLogger(__name__) @@ -175,12 +175,14 @@ async def _stale_tasks_monitor_worker(self) -> None: task_id, with_task_context=None, reraise_errors=False ) - def list_tasks(self, with_task_context: TaskContext | None) -> list[TrackedTask]: + def list_tasks(self, with_task_context: TaskContext | None) -> list[TaskBase]: if not with_task_context: - return list(self._tracked_tasks.values()) + return [ + TaskBase(task_id=task.task_id) for task in self._tracked_tasks.values() + ] return [ - task + TaskBase(task_id=task.task_id) for task in self._tracked_tasks.values() if task.task_context == with_task_context ] From cd38439f7760621325032ec866fcbe9bca93a536 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 13:30:25 +0200 Subject: [PATCH 20/33] refactor --- .../api_schemas_long_running_tasks/tasks.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py index 81a99dbf20d..e948628b9f5 100644 --- a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py +++ b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py @@ -22,7 +22,8 @@ class TaskBase(BaseModel): task_id: TaskId # NOTE: task name can always be extraced from the task_id - # since it'e encoded inside it + # since it'e encoded inside it (expect when this is ued + # with data coming form the celery tasks) task_name: str = "" @field_validator("task_name", mode="before") @@ -30,15 +31,15 @@ class TaskBase(BaseModel): def populate_task_name(cls, task_id, info): task_name = task_id - # attempt to solve the task name from the task_id + # attempt to extract the task name from the task_id # if this is coming form a long_running_task task_id = info.data.get("task_id") if task_id: parts = task_id.split(".") if len(parts) >= 2: - task_name = parts[1] + task_name = urllib.parse.unquote(parts[1]) - return urllib.parse.unquote(task_name) + return task_name class TaskGet(TaskBase): From bba46c8958e2ff3fad25782f461c33f7f25c970d Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 13:30:43 +0200 Subject: [PATCH 21/33] refactor --- .../src/models_library/api_schemas_long_running_tasks/tasks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py index e948628b9f5..e4822354d1c 100644 --- a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py +++ b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py @@ -29,7 +29,6 @@ class TaskBase(BaseModel): @field_validator("task_name", mode="before") @classmethod def populate_task_name(cls, task_id, info): - task_name = task_id # attempt to extract the task name from the task_id # if this is coming form a long_running_task From d6885b25e82a9507e439268e1a68daa6e87fa196 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 13:31:26 +0200 Subject: [PATCH 22/33] refactor --- .../src/models_library/api_schemas_long_running_tasks/tasks.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py index e4822354d1c..868fb85cf1f 100644 --- a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py +++ b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py @@ -28,8 +28,7 @@ class TaskBase(BaseModel): @field_validator("task_name", mode="before") @classmethod - def populate_task_name(cls, task_id, info): - + def populate_task_name(cls, task_name: str, info): # attempt to extract the task name from the task_id # if this is coming form a long_running_task task_id = info.data.get("task_id") From d61ba00a5b861503bdbb93ccdb39bd9b7c686dda Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 14:09:08 +0200 Subject: [PATCH 23/33] fixed broken job name --- services/web/server/src/simcore_service_webserver/tasks/_rest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/server/src/simcore_service_webserver/tasks/_rest.py b/services/web/server/src/simcore_service_webserver/tasks/_rest.py index c0795c8137c..3455f02156b 100644 --- a/services/web/server/src/simcore_service_webserver/tasks/_rest.py +++ b/services/web/server/src/simcore_service_webserver/tasks/_rest.py @@ -78,6 +78,7 @@ async def get_async_jobs(request: web.Request) -> web.Response: [ TaskGet( task_id=f"{job.job_id}", + task_name=job.job_name, status_href=f"{request.url.with_path(str(request.app.router['get_async_job_status'].url_for(task_id=str(job.job_id))))}", abort_href=f"{request.url.with_path(str(request.app.router['cancel_async_job'].url_for(task_id=str(job.job_id))))}", result_href=f"{request.url.with_path(str(request.app.router['get_async_job_result'].url_for(task_id=str(job.job_id))))}", From 888e10f31095100dba07c8afe14f311b4de5c8d7 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 13 Jun 2025 14:09:31 +0200 Subject: [PATCH 24/33] fixed validator --- .../api_schemas_long_running_tasks/tasks.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py index 868fb85cf1f..9bbe0670ec0 100644 --- a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py +++ b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py @@ -28,13 +28,13 @@ class TaskBase(BaseModel): @field_validator("task_name", mode="before") @classmethod - def populate_task_name(cls, task_name: str, info): + def populate_task_name_if_not_provided(cls, task_name: str, info): # attempt to extract the task name from the task_id # if this is coming form a long_running_task - task_id = info.data.get("task_id") - if task_id: + task_id = info.data["task_id"] + if task_id and task_name == "": parts = task_id.split(".") - if len(parts) >= 2: + if len(parts) > 1: task_name = urllib.parse.unquote(parts[1]) return task_name From c3e7129fdd8b0f9b483ea2729d19973343f9a505 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Mon, 16 Jun 2025 09:32:35 +0200 Subject: [PATCH 25/33] fixed broken --- .../api_schemas_long_running_tasks/tasks.py | 28 +++++----- ...st_api_schemas_long_running_tasks_tasks.py | 56 +++++++++++++++++++ 2 files changed, 69 insertions(+), 15 deletions(-) create mode 100644 packages/models-library/tests/test_api_schemas_long_running_tasks_tasks.py diff --git a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py index 9bbe0670ec0..997b4e12c3f 100644 --- a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py +++ b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py @@ -2,7 +2,7 @@ from datetime import datetime from typing import Any -from pydantic import BaseModel, field_validator +from pydantic import BaseModel, model_validator from .base import TaskId, TaskProgress @@ -20,24 +20,22 @@ class TaskResult(BaseModel): class TaskBase(BaseModel): task_id: TaskId - - # NOTE: task name can always be extraced from the task_id - # since it'e encoded inside it (expect when this is ued - # with data coming form the celery tasks) task_name: str = "" - @field_validator("task_name", mode="before") - @classmethod - def populate_task_name_if_not_provided(cls, task_name: str, info): - # attempt to extract the task name from the task_id - # if this is coming form a long_running_task - task_id = info.data["task_id"] - if task_id and task_name == "": - parts = task_id.split(".") + @model_validator(mode="after") + def try_populate_task_name_from_task_id(self) -> "TaskBase": + # NOTE: currently this model is used to validate tasks coming from + # the celery backend and form long_running_tasks + # 1. if a task comes from Celery, it will keep it's given name + # 2. if a task comes from long_running_tasks, it will extract it form + # the task_id, which looks like "{PREFIX}.{TASK_NAME}.UNIQUE|{UUID}" + + if self.task_id and self.task_name == "": + parts = self.task_id.split(".") if len(parts) > 1: - task_name = urllib.parse.unquote(parts[1]) + self.task_name = urllib.parse.unquote(parts[1]) - return task_name + return self class TaskGet(TaskBase): diff --git a/packages/models-library/tests/test_api_schemas_long_running_tasks_tasks.py b/packages/models-library/tests/test_api_schemas_long_running_tasks_tasks.py new file mode 100644 index 00000000000..aeeb1730d9c --- /dev/null +++ b/packages/models-library/tests/test_api_schemas_long_running_tasks_tasks.py @@ -0,0 +1,56 @@ +import pytest +from models_library.api_schemas_long_running_tasks.tasks import TaskGet +from pydantic import TypeAdapter + + +def _get_data_withut_task_name(task_id: str) -> dict: + return { + "task_id": task_id, + "status_href": "", + "result_href": "", + "abort_href": "", + } + + +@pytest.mark.parametrize( + "data, expected_task_name", + [ + (_get_data_withut_task_name("a.b.c.d"), "b"), + (_get_data_withut_task_name("a.b.c"), "b"), + (_get_data_withut_task_name("a.b"), "b"), + (_get_data_withut_task_name("a"), ""), + ], +) +def test_try_extract_task_name(data: dict, expected_task_name: str) -> None: + task_get = TaskGet(**data) + assert task_get.task_name == expected_task_name + + task_get = TypeAdapter(TaskGet).validate_python(data) + assert task_get.task_name == expected_task_name + + +def _get_data_with_task_name(task_id: str, task_name: str) -> dict: + return { + "task_id": task_id, + "task_name": task_name, + "status_href": "", + "result_href": "", + "abort_href": "", + } + + +@pytest.mark.parametrize( + "data, expected_task_name", + [ + (_get_data_with_task_name("a.b.c.d", "a_name"), "a_name"), + (_get_data_with_task_name("a.b.c", "a_name"), "a_name"), + (_get_data_with_task_name("a.b", "a_name"), "a_name"), + (_get_data_with_task_name("a", "a_name"), "a_name"), + ], +) +def test_task_name_is_provided(data: dict, expected_task_name: str) -> None: + task_get = TaskGet(**data) + assert task_get.task_name == expected_task_name + + task_get = TypeAdapter(TaskGet).validate_python(data) + assert task_get.task_name == expected_task_name From 479165bb8fc5ae3de7e86d002dfede2399c10faf Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Mon, 16 Jun 2025 10:05:43 +0200 Subject: [PATCH 26/33] revert change --- .../tests/unit/with_dbs/02/test_projects_cancellations.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_cancellations.py b/services/web/server/tests/unit/with_dbs/02/test_projects_cancellations.py index 4534d806a8e..2e92800de64 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_cancellations.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_cancellations.py @@ -170,6 +170,8 @@ async def test_copying_large_project_and_retrieving_copy_task( assert not error list_of_tasks = TypeAdapter(list[TaskGet]).validate_python(data) assert len(list_of_tasks) == 1 + task = list_of_tasks[0] + assert task.task_name == f"POST {create_url}" # let the copy start await asyncio.sleep(2) # now abort the copy From 1a4072151404bf55737deab9ab30cd0f2a033fdb Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Mon, 16 Jun 2025 10:14:55 +0200 Subject: [PATCH 27/33] fixed --- .../aiohttp/long_running_tasks/test_long_running_tasks.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py b/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py index 6d215e0ae8a..276af35d39c 100644 --- a/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py +++ b/packages/service-library/tests/aiohttp/long_running_tasks/test_long_running_tasks.py @@ -214,6 +214,13 @@ async def test_list_tasks( list_of_tasks = TypeAdapter(list[TaskGet]).validate_python(data) assert len(list_of_tasks) == NUM_TASKS + # the task name is properly formatted + assert all( + task.task_name.startswith( + "POST /long_running_task:start?num_strings=10&sleep_time=" + ) + for task in list_of_tasks + ) # now wait for them to finish await asyncio.gather(*(wait_for_task(client, task_id, {}) for task_id in results)) # now get the result one by one From 6a38a8b994f960aaf39034b9fa79531cc833af03 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 18 Jun 2025 10:58:21 +0200 Subject: [PATCH 28/33] feedback --- .../api_schemas_long_running_tasks/tasks.py | 12 +++++++++--- .../test_api_schemas_long_running_tasks_tasks.py | 10 +++++----- .../src/servicelib/long_running_tasks/errors.py | 2 +- .../src/servicelib/long_running_tasks/task.py | 2 +- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py index 997b4e12c3f..8f27127c71a 100644 --- a/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py +++ b/packages/models-library/src/models_library/api_schemas_long_running_tasks/tasks.py @@ -2,7 +2,8 @@ from datetime import datetime from typing import Any -from pydantic import BaseModel, model_validator +from common_library.exclude import Unset +from pydantic import BaseModel, ConfigDict, model_validator from .base import TaskId, TaskProgress @@ -20,7 +21,7 @@ class TaskResult(BaseModel): class TaskBase(BaseModel): task_id: TaskId - task_name: str = "" + task_name: str | Unset = Unset.VALUE @model_validator(mode="after") def try_populate_task_name_from_task_id(self) -> "TaskBase": @@ -30,13 +31,18 @@ def try_populate_task_name_from_task_id(self) -> "TaskBase": # 2. if a task comes from long_running_tasks, it will extract it form # the task_id, which looks like "{PREFIX}.{TASK_NAME}.UNIQUE|{UUID}" - if self.task_id and self.task_name == "": + if self.task_id and self.task_name == Unset.VALUE: parts = self.task_id.split(".") if len(parts) > 1: self.task_name = urllib.parse.unquote(parts[1]) + if self.task_name == Unset.VALUE: + self.task_name = self.task_id + return self + model_config = ConfigDict(arbitrary_types_allowed=True) + class TaskGet(TaskBase): status_href: str diff --git a/packages/models-library/tests/test_api_schemas_long_running_tasks_tasks.py b/packages/models-library/tests/test_api_schemas_long_running_tasks_tasks.py index aeeb1730d9c..5acdf168ccc 100644 --- a/packages/models-library/tests/test_api_schemas_long_running_tasks_tasks.py +++ b/packages/models-library/tests/test_api_schemas_long_running_tasks_tasks.py @@ -3,7 +3,7 @@ from pydantic import TypeAdapter -def _get_data_withut_task_name(task_id: str) -> dict: +def _get_data_without_task_name(task_id: str) -> dict: return { "task_id": task_id, "status_href": "", @@ -15,10 +15,10 @@ def _get_data_withut_task_name(task_id: str) -> dict: @pytest.mark.parametrize( "data, expected_task_name", [ - (_get_data_withut_task_name("a.b.c.d"), "b"), - (_get_data_withut_task_name("a.b.c"), "b"), - (_get_data_withut_task_name("a.b"), "b"), - (_get_data_withut_task_name("a"), ""), + (_get_data_without_task_name("a.b.c.d"), "b"), + (_get_data_without_task_name("a.b.c"), "b"), + (_get_data_without_task_name("a.b"), "b"), + (_get_data_without_task_name("a"), "a"), ], ) def test_try_extract_task_name(data: dict, expected_task_name: str) -> None: diff --git a/packages/service-library/src/servicelib/long_running_tasks/errors.py b/packages/service-library/src/servicelib/long_running_tasks/errors.py index e199a3128ce..75e46da5b0c 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/errors.py +++ b/packages/service-library/src/servicelib/long_running_tasks/errors.py @@ -7,7 +7,7 @@ class BaseLongRunningError(OsparcErrorMixin, Exception): class TaskNotRegisteredError(BaseLongRunningError): msg_template: str = ( - "notask with task_name='{task_name}' was found in the task registry. " + "no task with task_name='{task_name}' was found in the task registry. " "Make sure it's registered before starting it." ) diff --git a/packages/service-library/src/servicelib/long_running_tasks/task.py b/packages/service-library/src/servicelib/long_running_tasks/task.py index d15622dc67a..c3ccbc270bc 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/task.py +++ b/packages/service-library/src/servicelib/long_running_tasks/task.py @@ -30,7 +30,7 @@ # NOTE: for now only this one is used, in future it will be unqiue per service # and this default will be removed and become mandatory -_DEFAULT_NAMESPACE: Final = "lrt" +_DEFAULT_NAMESPACE: Final[str] = "lrt" _CANCEL_TASK_TIMEOUT: Final[PositiveFloat] = datetime.timedelta( seconds=1 From 4ba350932cea4b11c26603086c7d6afc3fbfccb3 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 18 Jun 2025 11:03:20 +0200 Subject: [PATCH 29/33] fixed merge --- .../http_endpoint_responses.py | 63 ------------------- .../servicelib/long_running_tasks/lrt_api.py | 31 +++++++-- 2 files changed, 26 insertions(+), 68 deletions(-) delete mode 100644 packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py diff --git a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py b/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py deleted file mode 100644 index 5256849541b..00000000000 --- a/packages/service-library/src/servicelib/long_running_tasks/http_endpoint_responses.py +++ /dev/null @@ -1,63 +0,0 @@ -import logging -from typing import Any - -from common_library.error_codes import create_error_code -from servicelib.logging_errors import create_troubleshotting_log_kwargs - -from .errors import TaskNotCompletedError, TaskNotFoundError -from .models import TaskBase, TaskId, TaskStatus -from .task import TaskContext, TasksManager, TrackedTask - -_logger = logging.getLogger(__name__) - - -def list_tasks( - tasks_manager: TasksManager, task_context: TaskContext | None -) -> list[TaskBase]: - tracked_tasks: list[TrackedTask] = tasks_manager.list_tasks( - with_task_context=task_context - ) - return [TaskBase(task_id=t.task_id, task_name=t.task_name) for t in tracked_tasks] - - -def get_task_status( - tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId -) -> TaskStatus: - return tasks_manager.get_task_status( - task_id=task_id, with_task_context=task_context - ) - - -async def get_task_result( - tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId -) -> Any: - try: - task_result = tasks_manager.get_task_result( - task_id, with_task_context=task_context - ) - await tasks_manager.remove_task( - task_id, with_task_context=task_context, reraise_errors=False - ) - return task_result - except (TaskNotFoundError, TaskNotCompletedError): - raise - except Exception as exc: - _logger.exception( - **create_troubleshotting_log_kwargs( - user_error_msg=f"{task_id=} raised an exception", - error=exc, - error_code=create_error_code(exc), - error_context={"task_context": task_context, "task_id": task_id}, - ), - ) - # the task shall be removed in this case - await tasks_manager.remove_task( - task_id, with_task_context=task_context, reraise_errors=False - ) - raise - - -async def remove_task( - tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId -) -> None: - await tasks_manager.remove_task(task_id, with_task_context=task_context) diff --git a/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py b/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py index dbda9376aca..7965e7eef11 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py +++ b/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py @@ -1,8 +1,15 @@ +import logging from typing import Any +from common_library.error_codes import create_error_code +from servicelib.logging_errors import create_troubleshotting_log_kwargs + +from .errors import TaskNotCompletedError, TaskNotFoundError from .models import TaskBase, TaskId, TaskStatus from .task import RegisteredTaskName, TaskContext, TasksManager +_logger = logging.getLogger(__name__) + async def start_task( tasks_manager: TasksManager, @@ -67,16 +74,30 @@ def get_task_status( async def get_task_result( tasks_manager: TasksManager, task_context: TaskContext | None, task_id: TaskId ) -> Any: - """retruns the result of a task, which is directly whatever the remove hanlder returned""" try: - return tasks_manager.get_task_result( - task_id=task_id, with_task_context=task_context + task_result = tasks_manager.get_task_result( + task_id, with_task_context=task_context + ) + await tasks_manager.remove_task( + task_id, with_task_context=task_context, reraise_errors=False + ) + return task_result + except (TaskNotFoundError, TaskNotCompletedError): + raise + except Exception as exc: + _logger.exception( + **create_troubleshotting_log_kwargs( + user_error_msg=f"{task_id=} raised an exception", + error=exc, + error_code=create_error_code(exc), + error_context={"task_context": task_context, "task_id": task_id}, + ), ) - finally: - # the task is always removed even if an error occurs + # the task shall be removed in this case await tasks_manager.remove_task( task_id, with_task_context=task_context, reraise_errors=False ) + raise async def remove_task( From 07c905fadb26a91dff179401f3d738dd4a62cb76 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 18 Jun 2025 11:36:01 +0200 Subject: [PATCH 30/33] updated specs --- .../web/server/src/simcore_service_webserver/api/v0/openapi.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 5c611a7932a..a20e23c76d0 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -16754,7 +16754,6 @@ components: task_name: type: string title: Task Name - default: '' status_href: type: string title: Status Href From d2b21f8d692264db7740633675cc647c8a3064e4 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 16 Jul 2025 15:17:39 +0200 Subject: [PATCH 31/33] mege conflict edits --- packages/service-library/src/servicelib/logging_errors.py | 2 +- .../src/servicelib/long_running_tasks/lrt_api.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/service-library/src/servicelib/logging_errors.py b/packages/service-library/src/servicelib/logging_errors.py index 3099aa9b863..938a4c3f62d 100644 --- a/packages/service-library/src/servicelib/logging_errors.py +++ b/packages/service-library/src/servicelib/logging_errors.py @@ -74,7 +74,7 @@ def create_troubleshootting_log_kwargs( ... except MyException as exc _logger.exception( - **create_troubleshotting_log_kwargs( + **create_troubleshootting_log_kwargs( user_error_msg=frontend_msg, error=exc, error_context={ diff --git a/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py b/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py index 7965e7eef11..007d2db0715 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py +++ b/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py @@ -2,7 +2,7 @@ from typing import Any from common_library.error_codes import create_error_code -from servicelib.logging_errors import create_troubleshotting_log_kwargs +from servicelib.logging_errors import create_troubleshootting_log_kwargs from .errors import TaskNotCompletedError, TaskNotFoundError from .models import TaskBase, TaskId, TaskStatus @@ -86,8 +86,8 @@ async def get_task_result( raise except Exception as exc: _logger.exception( - **create_troubleshotting_log_kwargs( - user_error_msg=f"{task_id=} raised an exception", + **create_troubleshootting_log_kwargs( + user_error_msg=f"{task_id=} raised an exception while handling", error=exc, error_code=create_error_code(exc), error_context={"task_context": task_context, "task_id": task_id}, From 066db30ed58e1a41f33e0f1b0efd402f39b8a732 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Wed, 16 Jul 2025 15:18:48 +0200 Subject: [PATCH 32/33] using previous message --- .../src/servicelib/long_running_tasks/lrt_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py b/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py index 007d2db0715..5e8c9191f7e 100644 --- a/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py +++ b/packages/service-library/src/servicelib/long_running_tasks/lrt_api.py @@ -87,7 +87,7 @@ async def get_task_result( except Exception as exc: _logger.exception( **create_troubleshootting_log_kwargs( - user_error_msg=f"{task_id=} raised an exception while handling", + user_error_msg=f"{task_id=} raised an exception while getting its result", error=exc, error_code=create_error_code(exc), error_context={"task_context": task_context, "task_id": task_id}, From 2a761985f7a2f9ebef14bbd9112f0c74a5660bf7 Mon Sep 17 00:00:00 2001 From: Andrei Neagu Date: Fri, 18 Jul 2025 06:36:22 +0200 Subject: [PATCH 33/33] updated specs --- services/api-server/openapi.json | 1 - 1 file changed, 1 deletion(-) diff --git a/services/api-server/openapi.json b/services/api-server/openapi.json index 40b9594f3e5..4598a9b45d9 100644 --- a/services/api-server/openapi.json +++ b/services/api-server/openapi.json @@ -11480,7 +11480,6 @@ "type": "object", "required": [ "task_id", - "task_name", "status_href", "result_href", "abort_href"