From 3bca5c9c9015e43c589eee33d4547f0b7db78af7 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 14 Jan 2025 14:23:54 +0100 Subject: [PATCH 01/28] refactor --- .../src/servicelib/project_lock.py | 9 ++++++++- .../services/background_tasks.py | 12 ++---------- .../simcore_service_webserver/projects/lock.py | 16 ++++++---------- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/packages/service-library/src/servicelib/project_lock.py b/packages/service-library/src/servicelib/project_lock.py index f2ae6ce6ddd..66081474111 100644 --- a/packages/service-library/src/servicelib/project_lock.py +++ b/packages/service-library/src/servicelib/project_lock.py @@ -11,6 +11,7 @@ from models_library.projects_access import Owner from models_library.projects_state import ProjectLocked, ProjectStatus from redis.asyncio.lock import Lock +from servicelib.redis._client import RedisClientSDK from .background_task import periodic_task from .logging_utils import log_context @@ -31,7 +32,8 @@ async def _auto_extend_project_lock(project_lock: Lock) -> None: @asynccontextmanager async def lock_project( - redis_lock: Lock, + redis_client: RedisClientSDK, + *, project_uuid: str | ProjectID, status: ProjectStatus, owner: Owner | None = None, @@ -42,6 +44,11 @@ async def lock_project( ProjectLockError: if project is already locked """ + redis_lock = redis_client.create_lock( + PROJECT_REDIS_LOCK_KEY.format(project_uuid), + ttl=PROJECT_LOCK_TIMEOUT, + ) + try: if not await redis_lock.acquire( blocking=False, diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py index b569f679f19..a07d498d02b 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py @@ -5,11 +5,7 @@ from models_library.projects import ProjectID from models_library.projects_state import ProjectStatus from servicelib.logging_utils import log_context -from servicelib.project_lock import ( - PROJECT_LOCK_TIMEOUT, - PROJECT_REDIS_LOCK_KEY, - lock_project, -) +from servicelib.project_lock import lock_project from simcore_postgres_database.utils_projects import ( DBProjectNotFoundError, ProjectsRepo, @@ -61,12 +57,8 @@ async def removal_policy_task(app: FastAPI) -> None: logging.INFO, msg=f"Removing data for project {project_id} started, project last change date {_project_last_change_date}, efs removal policy task age limit timedelta {app_settings.EFS_REMOVAL_POLICY_TASK_AGE_LIMIT_TIMEDELTA}", ): - redis_lock = get_redis_lock_client(app).create_lock( - PROJECT_REDIS_LOCK_KEY.format(project_id), - ttl=PROJECT_LOCK_TIMEOUT, - ) async with lock_project( - redis_lock, + get_redis_lock_client(app), project_uuid=project_id, status=ProjectStatus.MAINTAINING, ): diff --git a/services/web/server/src/simcore_service_webserver/projects/lock.py b/services/web/server/src/simcore_service_webserver/projects/lock.py index 84b24c087e7..53072870e3d 100644 --- a/services/web/server/src/simcore_service_webserver/projects/lock.py +++ b/services/web/server/src/simcore_service_webserver/projects/lock.py @@ -6,10 +6,10 @@ from models_library.projects import ProjectID from models_library.projects_access import Owner from models_library.projects_state import ProjectLocked, ProjectStatus -from servicelib.project_lock import PROJECT_LOCK_TIMEOUT, PROJECT_REDIS_LOCK_KEY +from servicelib.project_lock import PROJECT_REDIS_LOCK_KEY from servicelib.project_lock import lock_project as common_lock_project -from ..redis import get_redis_lock_manager_client +from ..redis import get_redis_lock_manager_client, get_redis_lock_manager_client_sdk from ..users.api import FullNameDict _logger = logging.getLogger(__name__) @@ -28,15 +28,11 @@ async def lock_project( Raises: ProjectLockError: if project is already locked """ - - redis_lock = get_redis_lock_manager_client(app).lock( - PROJECT_REDIS_LOCK_KEY.format(project_uuid), - timeout=PROJECT_LOCK_TIMEOUT.total_seconds(), - ) - owner = Owner(user_id=user_id, **user_fullname) - async with common_lock_project( - redis_lock, project_uuid=project_uuid, status=status, owner=owner + get_redis_lock_manager_client_sdk(app), + project_uuid=project_uuid, + status=status, + owner=Owner(user_id=user_id, **user_fullname), ): yield From 065ca4dd540b46e98c517ad27397023d04bdf43d Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 14 Jan 2025 15:47:25 +0100 Subject: [PATCH 02/28] ongoing changes --- .../src/servicelib/project_lock.py | 84 ++++++------------- .../services/background_tasks.py | 26 ++++-- 2 files changed, 43 insertions(+), 67 deletions(-) diff --git a/packages/service-library/src/servicelib/project_lock.py b/packages/service-library/src/servicelib/project_lock.py index 66081474111..ab9770de27f 100644 --- a/packages/service-library/src/servicelib/project_lock.py +++ b/packages/service-library/src/servicelib/project_lock.py @@ -1,87 +1,51 @@ import datetime -import logging -from asyncio.log import logger -from collections.abc import AsyncIterator -from contextlib import asynccontextmanager -from typing import Final, TypeAlias +import functools +from collections.abc import Callable, Coroutine +from typing import Any, Final, ParamSpec, TypeAlias, TypeVar import redis import redis.exceptions from models_library.projects import ProjectID from models_library.projects_access import Owner from models_library.projects_state import ProjectLocked, ProjectStatus -from redis.asyncio.lock import Lock -from servicelib.redis._client import RedisClientSDK -from .background_task import periodic_task -from .logging_utils import log_context - -_logger = logging.getLogger(__name__) +from .redis import RedisClientSDK, exclusive PROJECT_REDIS_LOCK_KEY: str = "project_lock:{}" PROJECT_LOCK_TIMEOUT: Final[datetime.timedelta] = datetime.timedelta(seconds=10) -ProjectLock = Lock ProjectLockError: TypeAlias = redis.exceptions.LockError -async def _auto_extend_project_lock(project_lock: Lock) -> None: - # NOTE: the background task already catches anything that might raise here - await project_lock.reacquire() +P = ParamSpec("P") +R = TypeVar("R") -@asynccontextmanager -async def lock_project( - redis_client: RedisClientSDK, +def with_locked_project( + redis_client: RedisClientSDK | Callable[..., RedisClientSDK], *, project_uuid: str | ProjectID, status: ProjectStatus, owner: Owner | None = None, -) -> AsyncIterator[None]: - """Context manager to lock and unlock a project by user_id - - Raises: - ProjectLockError: if project is already locked - """ - - redis_lock = redis_client.create_lock( - PROJECT_REDIS_LOCK_KEY.format(project_uuid), - ttl=PROJECT_LOCK_TIMEOUT, - ) - - try: - if not await redis_lock.acquire( - blocking=False, - token=ProjectLocked( +) -> Callable[ + [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] +]: + def _decorator( + func: Callable[P, Coroutine[Any, Any, R]], + ) -> Callable[P, Coroutine[Any, Any, R]]: + @exclusive( + redis_client, + lock_key=PROJECT_REDIS_LOCK_KEY.format(project_uuid), + lock_value=ProjectLocked( value=True, owner=owner, status=status, ).model_dump_json(), - ): - msg = f"Lock for project {project_uuid!r} owner {owner!r} could not be acquired" - raise ProjectLockError(msg) + ) + @functools.wraps(func) + async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: + return await func(*args, **kwargs) - with log_context( - _logger, - logging.DEBUG, - msg=f"with lock for {owner=}:{project_uuid=}:{status=}", - ): - async with periodic_task( - _auto_extend_project_lock, - interval=0.6 * PROJECT_LOCK_TIMEOUT, - task_name=f"{PROJECT_REDIS_LOCK_KEY.format(project_uuid)}_lock_auto_extend", - project_lock=redis_lock, - ): - yield + return _wrapper - finally: - # let's ensure we release that stuff - try: - if await redis_lock.owned(): - await redis_lock.release() - except (redis.exceptions.LockError, redis.exceptions.LockNotOwnedError) as exc: - logger.warning( - "releasing %s unexpectedly raised an exception: %s", - f"{redis_lock=!r}", - f"{exc}", - ) + return _decorator diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py index a07d498d02b..e49e5a31348 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py @@ -5,7 +5,7 @@ from models_library.projects import ProjectID from models_library.projects_state import ProjectStatus from servicelib.logging_utils import log_context -from servicelib.project_lock import lock_project +from servicelib.project_lock import with_locked_project from simcore_postgres_database.utils_projects import ( DBProjectNotFoundError, ProjectsRepo, @@ -18,6 +18,23 @@ _logger = logging.getLogger(__name__) +async def _remove_data_with_lock(app: FastAPI, project_id: ProjectID) -> None: + # Decorate a new function that will call the necessary coroutine + efs_manager: EfsManager = app.state.efs_manager + + @with_locked_project( + get_redis_lock_client(app), + project_uuid=project_id, + status=ProjectStatus.MAINTAINING, + ) + async def _remove(): + # Call the actual coroutine function + await efs_manager.remove_project_efs_data(project_id) + + # Execute the decorated function + await _remove() + + async def removal_policy_task(app: FastAPI) -> None: _logger.info("Removal policy task started") @@ -57,9 +74,4 @@ async def removal_policy_task(app: FastAPI) -> None: logging.INFO, msg=f"Removing data for project {project_id} started, project last change date {_project_last_change_date}, efs removal policy task age limit timedelta {app_settings.EFS_REMOVAL_POLICY_TASK_AGE_LIMIT_TIMEDELTA}", ): - async with lock_project( - get_redis_lock_client(app), - project_uuid=project_id, - status=ProjectStatus.MAINTAINING, - ): - await efs_manager.remove_project_efs_data(project_id) + await _remove_data_with_lock(app, project_id) From 40f6e4f2313869ea294a6295a51ede4eaecdf4cb Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 14 Jan 2025 17:33:22 +0100 Subject: [PATCH 03/28] ruff --- services/efs-guardian/tests/unit/conftest.py | 33 +++----------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/services/efs-guardian/tests/unit/conftest.py b/services/efs-guardian/tests/unit/conftest.py index 61d2daaba6d..7aaa50f616b 100644 --- a/services/efs-guardian/tests/unit/conftest.py +++ b/services/efs-guardian/tests/unit/conftest.py @@ -17,7 +17,6 @@ from asgi_lifespan import LifespanManager from faker import Faker from fastapi import FastAPI -from httpx._transports.asgi import ASGITransport from pytest_mock import MockerFixture from pytest_simcore.helpers.typing_env import EnvVarsDict from servicelib.rabbitmq import RabbitMQRPCClient @@ -119,38 +118,17 @@ async def client(app: FastAPI) -> AsyncIterator[httpx.AsyncClient]: # - Needed for app to trigger start/stop event handlers # - Prefer this client instead of fastapi.testclient.TestClient async with httpx.AsyncClient( - app=app, - base_url="http://efs-guardian.testserver.io", + transport=httpx.ASGITransport(app=app), + base_url=f"http://{app.title}.testserver.io", headers={"Content-Type": "application/json"}, ) as client: - assert isinstance( - client._transport, ASGITransport # pylint: disable=protected-access - ) yield client -# -# Redis -# - - -@pytest.fixture -def disable_redis_and_background_tasks_setup(mocker: MockerFixture) -> Callable: - def _(): - # The following services are affected if redis is not in place - mocker.patch("simcore_service_efs_guardian.core.application.setup_redis") - mocker.patch( - "simcore_service_efs_guardian.core.application.setup_background_tasks" - ) - - return _ - - @pytest.fixture -def with_disabled_redis_and_background_tasks( - disable_redis_and_background_tasks_setup: Callable, -): - disable_redis_and_background_tasks_setup() +def with_disabled_redis_and_background_tasks(mocker: MockerFixture) -> None: + mocker.patch("simcore_service_efs_guardian.core.application.setup_redis") + mocker.patch("simcore_service_efs_guardian.core.application.setup_background_tasks") # @@ -160,7 +138,6 @@ def with_disabled_redis_and_background_tasks( @pytest.fixture async def efs_cleanup(app: FastAPI): - yield aws_efs_settings: AwsEfsSettings = app.state.settings.EFS_GUARDIAN_AWS_EFS_SETTINGS From 9be8b5cdf847a9c820af9ec5c6f4159a4b72d529 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 14 Jan 2025 17:52:54 +0100 Subject: [PATCH 04/28] fixed test --- services/efs-guardian/tests/unit/conftest.py | 7 ++++- .../unit/test_efs_removal_policy_task.py | 28 +++++++++++++++---- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/services/efs-guardian/tests/unit/conftest.py b/services/efs-guardian/tests/unit/conftest.py index 7aaa50f616b..d598fe06ebd 100644 --- a/services/efs-guardian/tests/unit/conftest.py +++ b/services/efs-guardian/tests/unit/conftest.py @@ -125,6 +125,11 @@ async def client(app: FastAPI) -> AsyncIterator[httpx.AsyncClient]: yield client +@pytest.fixture +def with_disabled_background_tasks(mocker: MockerFixture) -> None: + mocker.patch("simcore_service_efs_guardian.core.application.setup_background_tasks") + + @pytest.fixture def with_disabled_redis_and_background_tasks(mocker: MockerFixture) -> None: mocker.patch("simcore_service_efs_guardian.core.application.setup_redis") @@ -137,7 +142,7 @@ def with_disabled_redis_and_background_tasks(mocker: MockerFixture) -> None: @pytest.fixture -async def efs_cleanup(app: FastAPI): +async def efs_cleanup(app: FastAPI) -> AsyncIterator[None]: yield aws_efs_settings: AwsEfsSettings = app.state.settings.EFS_GUARDIAN_AWS_EFS_SETTINGS diff --git a/services/efs-guardian/tests/unit/test_efs_removal_policy_task.py b/services/efs-guardian/tests/unit/test_efs_removal_policy_task.py index cd57d865002..30da8fd8cf8 100644 --- a/services/efs-guardian/tests/unit/test_efs_removal_policy_task.py +++ b/services/efs-guardian/tests/unit/test_efs_removal_policy_task.py @@ -102,10 +102,28 @@ async def project_in_db( yield row +# Create a mock object manually +mock_with_locked_project = MagicMock() + + +# The stand-in decorator to replace the original one and record the function call +def mock_decorator(*args, **kwargs): + def _decorator(func): + def wrapper(*args, **kwargs): + mock_with_locked_project(*args, **kwargs) # Log the call + return func(*args, **kwargs) + + return wrapper + + return _decorator + + @patch("simcore_service_efs_guardian.services.background_tasks.get_redis_lock_client") -@patch("simcore_service_efs_guardian.services.background_tasks.lock_project") +@patch( + "simcore_service_efs_guardian.services.background_tasks.with_locked_project", + new=mock_decorator, +) async def test_efs_removal_policy_task( - mock_lock_project: MagicMock, mock_get_redis_lock_client: MagicMock, faker: Faker, app: FastAPI, @@ -116,7 +134,7 @@ async def test_efs_removal_policy_task( ): # 1. Nothing should happen await removal_policy_task(app) - assert not mock_lock_project.called + assert not mock_with_locked_project.called # 2. Lets create some project with data aws_efs_settings: AwsEfsSettings = app.state.settings.EFS_GUARDIAN_AWS_EFS_SETTINGS @@ -148,7 +166,7 @@ async def test_efs_removal_policy_task( # 3. Nothing should happen await removal_policy_task(app) - assert not mock_lock_project.called + assert not mock_with_locked_project.called # 4. We will artifically change the project last change date app_settings: ApplicationSettings = app.state.settings @@ -169,7 +187,7 @@ async def test_efs_removal_policy_task( # 5. Now removal policy should remove those data await removal_policy_task(app) - assert mock_lock_project.assert_called_once + assert mock_with_locked_project.assert_called_once assert mock_get_redis_lock_client.assert_called_once projects_list = await efs_manager.list_projects_across_whole_efs() assert projects_list == [] From f36db6f09852306827aa4498e224a94e15cb52fe Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 14 Jan 2025 22:10:38 +0100 Subject: [PATCH 05/28] 1st trial --- .../exporter/_handlers.py | 42 ++--- .../projects/_crud_api_create.py | 25 +-- .../projects/lock.py | 46 +++--- .../projects/projects_api.py | 150 ++++++++++-------- .../unit/with_dbs/02/test_project_lock.py | 97 ++++++----- .../02/test_projects_crud_handlers__delete.py | 9 +- 6 files changed, 210 insertions(+), 159 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py index 97749637f54..63ff343fd06 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py @@ -1,6 +1,7 @@ import logging from collections.abc import Callable, Coroutine from contextlib import AsyncExitStack +from pathlib import Path from typing import Any from aiofiles.tempfile import TemporaryDirectory as AioTemporaryDirectory @@ -11,8 +12,7 @@ from .._constants import RQ_PRODUCT_KEY from .._meta import API_VTAG from ..login.decorators import login_required -from ..projects.lock import lock_project -from ..projects.projects_api import retrieve_and_notify_project_locked_state +from ..projects.projects_api import with_project_locked_notified_state from ..security.decorators import permission_required from ..users.api import get_user_fullname from ._formatter.archive import get_sds_archive_path @@ -42,18 +42,18 @@ async def export_project(request: web.Request): user_id = request[RQT_USERID_KEY] project_uuid = request.match_info.get("project_id") assert project_uuid # nosec - delete_tmp_dir: Callable[[], Coroutine[Any, Any, None]] | None = None - try: - async with AsyncExitStack() as tmp_dir_stack, lock_project( - request.app, - project_uuid, - ProjectStatus.EXPORTING, - user_id, - await get_user_fullname(request.app, user_id=user_id), - ): - await retrieve_and_notify_project_locked_state( - user_id, project_uuid, request.app - ) + + @with_project_locked_notified_state( + request.app, + project_uuid=project_uuid, + status=ProjectStatus.EXPORTING, + user_id=user_id, + user_name=await get_user_fullname(request.app, user_id=user_id), + notify_users=True, + ) + async def _() -> tuple[Callable[[], Coroutine[Any, Any, None]], Path]: + # @GitHK what is this supposed to be doing?? + async with AsyncExitStack() as tmp_dir_stack: tmp_dir = await tmp_dir_stack.enter_async_context(AioTemporaryDirectory()) file_to_download = await get_sds_archive_path( app=request.app, @@ -68,14 +68,14 @@ async def export_project(request: web.Request): msg = f"Must provide a file to download, not {file_to_download!s}" raise SDSException(msg) # this allows to transfer deletion of the tmp dir responsibility - delete_tmp_dir = tmp_dir_stack.pop_all().aclose - finally: - await retrieve_and_notify_project_locked_state( - user_id, project_uuid, request.app - ) + return tmp_dir_stack.pop_all().aclose, file_to_download + + delete_tmp_dir_callable, file_to_download = await _() headers = {"Content-Disposition": f'attachment; filename="{file_to_download.name}"'} - assert delete_tmp_dir # nosec + assert delete_tmp_dir_callable # nosec return CleanupFileResponse( - remove_tmp_dir_cb=delete_tmp_dir, path=file_to_download, headers=headers + remove_tmp_dir_cb=delete_tmp_dir_callable, + path=file_to_download, + headers=headers, ) 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 8b656f67cac..e72c921ea66 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 @@ -1,7 +1,6 @@ import asyncio import logging from collections.abc import Coroutine -from contextlib import AsyncExitStack from typing import Any, TypeAlias from aiohttp import web @@ -163,17 +162,7 @@ async def _copy_files_from_source_project( != ProjectTypeDB.TEMPLATE ) - async with AsyncExitStack() as stack: - if needs_lock_source_project: - await stack.enter_async_context( - projects_api.lock_with_notification( - app, - source_project["uuid"], - ProjectStatus.CLONING, - user_id, - await get_user_fullname(app, user_id=user_id), - ) - ) + async def _copy() -> None: starting_value = task_progress.percent async for long_running_task in copy_data_folders_from_project( app, source_project, new_project, nodes_map, user_id @@ -190,6 +179,18 @@ async def _copy_files_from_source_project( if long_running_task.done(): await long_running_task.result() + if needs_lock_source_project: + await projects_api.with_project_locked_notified_state( + app, + project_uuid=source_project["uuid"], + status=ProjectStatus.CLONING, + user_id=user_id, + user_name=await get_user_fullname(app, user_id=user_id), + notify_users=True, + )(_copy)() + else: + await _copy() + async def _compose_project_data( app: web.Application, diff --git a/services/web/server/src/simcore_service_webserver/projects/lock.py b/services/web/server/src/simcore_service_webserver/projects/lock.py index 53072870e3d..81e4987c918 100644 --- a/services/web/server/src/simcore_service_webserver/projects/lock.py +++ b/services/web/server/src/simcore_service_webserver/projects/lock.py @@ -1,40 +1,46 @@ -import logging -from collections.abc import AsyncIterator -from contextlib import asynccontextmanager +from collections.abc import Callable, Coroutine +from functools import wraps +from typing import Any, ParamSpec, TypeVar from aiohttp import web from models_library.projects import ProjectID from models_library.projects_access import Owner from models_library.projects_state import ProjectLocked, ProjectStatus -from servicelib.project_lock import PROJECT_REDIS_LOCK_KEY -from servicelib.project_lock import lock_project as common_lock_project +from servicelib.project_lock import PROJECT_REDIS_LOCK_KEY, with_locked_project from ..redis import get_redis_lock_manager_client, get_redis_lock_manager_client_sdk from ..users.api import FullNameDict -_logger = logging.getLogger(__name__) +P = ParamSpec("P") +R = TypeVar("R") -@asynccontextmanager -async def lock_project( +def with_locked_project_from_app( app: web.Application, + *, project_uuid: str | ProjectID, status: ProjectStatus, user_id: int, user_fullname: FullNameDict, -) -> AsyncIterator[None]: - """Context manager to lock and unlock a project by user_id +) -> Callable[ + [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] +]: + def _decorator( + func: Callable[P, Coroutine[Any, Any, R]], + ) -> Callable[P, Coroutine[Any, Any, R]]: + @with_locked_project( + get_redis_lock_manager_client_sdk(app), + project_uuid=project_uuid, + status=status, + owner=Owner(user_id=user_id, **user_fullname), + ) + @wraps(func) + async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: + return await func(*args, **kwargs) - Raises: - ProjectLockError: if project is already locked - """ - async with common_lock_project( - get_redis_lock_manager_client_sdk(app), - project_uuid=project_uuid, - status=status, - owner=Owner(user_id=user_id, **user_fullname), - ): - yield + return _wrapper + + return _decorator async def is_project_locked( diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index fa46afffed2..c3c3b2ca6f5 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -9,16 +9,16 @@ import asyncio import collections -import contextlib import datetime import json import logging from collections import defaultdict -from collections.abc import Generator +from collections.abc import Callable, Coroutine, Generator from contextlib import suppress from decimal import Decimal +from functools import wraps from pprint import pformat -from typing import Any, Final, cast +from typing import Any, Final, ParamSpec, TypeVar, cast from uuid import UUID, uuid4 from aiohttp import web @@ -145,7 +145,11 @@ ProjectStartsTooManyDynamicNodesError, ProjectTooManyProjectOpenedError, ) -from .lock import get_project_locked_state, is_project_locked, lock_project +from .lock import ( + get_project_locked_state, + is_project_locked, + with_locked_project_from_app, +) from .models import ProjectDict, ProjectPatchExtended from .settings import ProjectsSettings, get_plugin_settings from .utils import extract_dns_without_default_port @@ -361,8 +365,8 @@ async def _get_default_pricing_and_hardware_info( service_pricing_plan_get = await rut_api.get_default_service_pricing_plan( app, product_name=product_name, - service_key=ServiceKey(service_key), - service_version=ServiceVersion(service_version), + service_key=service_key, + service_version=service_version, ) if service_pricing_plan_get.pricing_units: for unit in service_pricing_plan_get.pricing_units: @@ -1256,14 +1260,16 @@ async def try_open_project_for_user( False if cannot be opened (e.g. locked, ) """ try: - async with lock_with_notification( + + @with_project_locked_notified_state( app, - project_uuid, - ProjectStatus.OPENING, - user_id, - await get_user_fullname(app, user_id=user_id), + project_uuid=project_uuid, + status=ProjectStatus.OPENING, + user_id=user_id, + user_name=await get_user_fullname(app, user_id=user_id), notify_users=False, - ): + ) + async def _open_project() -> bool: with managed_resource(user_id, client_session_id, app) as user_session: # NOTE: if max_number_of_studies_per_user is set, the same # project shall still be openable if the tab was closed @@ -1316,6 +1322,8 @@ async def try_open_project_for_user( return False + return await _open_project() + except ProjectLockError: # the project is currently locked return False @@ -1495,26 +1503,27 @@ async def add_project_states_for_user( lock_state = await _get_project_lock_state(user_id, project["uuid"], app) running_state = RunningState.UNKNOWN - if not is_template: - if computation_task := await director_v2_api.get_computation_task( + if not is_template and ( + computation_task := await director_v2_api.get_computation_task( app, user_id, project["uuid"] - ): - # get the running state - running_state = computation_task.state - # get the nodes individual states - for ( - node_id, - node_state, - ) in computation_task.pipeline_details.node_states.items(): - prj_node = project["workbench"].get(str(node_id)) - if prj_node is None: - continue - node_state_dict = json.loads( - node_state.model_dump_json(by_alias=True, exclude_unset=True) - ) - prj_node.setdefault("state", {}).update(node_state_dict) - prj_node_progress = node_state_dict.get("progress", None) or 0 - prj_node.update({"progress": round(prj_node_progress * 100.0)}) + ) + ): + # get the running state + running_state = computation_task.state + # get the nodes individual states + for ( + node_id, + node_state, + ) in computation_task.pipeline_details.node_states.items(): + prj_node = project["workbench"].get(str(node_id)) + if prj_node is None: + continue + node_state_dict = json.loads( + node_state.model_dump_json(by_alias=True, exclude_unset=True) + ) + prj_node.setdefault("state", {}).update(node_state_dict) + prj_node_progress = node_state_dict.get("progress", None) or 0 + prj_node.update({"progress": round(prj_node_progress * 100.0)}) project["state"] = ProjectState( locked=lock_state, state=ProjectRunningState(value=running_state) @@ -1748,14 +1757,15 @@ async def remove_project_dynamic_services( save_state = False # ------------------- - async with lock_with_notification( + @with_project_locked_notified_state( app, - project_uuid, - ProjectStatus.CLOSING, - user_id, - user_name_data, + project_uuid=project_uuid, + status=ProjectStatus.CLOSING, + user_id=user_id, + user_name=user_name_data, notify_users=notify_users, - ): + ) + async def _locked_stop_dynamic_serivces_in_project() -> None: # save the state if the user is not a guest. if we do not know we save in any case. with suppress( RPCServerError, @@ -1771,6 +1781,8 @@ async def remove_project_dynamic_services( save_state=save_state, ) + await _locked_stop_dynamic_serivces_in_project() + # # NOTIFICATIONS & LOCKS ----------------------------------------------------- @@ -1848,43 +1860,51 @@ async def retrieve_and_notify_project_locked_state( ) -@contextlib.asynccontextmanager -async def lock_with_notification( +P = ParamSpec("P") +R = TypeVar("R") + + +def with_project_locked_notified_state( app: web.Application, + *, project_uuid: str, status: ProjectStatus, user_id: int, user_name: FullNameDict, - *, - notify_users: bool = True, -): - try: - async with lock_project( - app, - project_uuid, - status, - user_id, - user_name, - ): + notify_users: bool, +) -> Callable[ + [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] +]: + def _decorator( + func: Callable[P, Coroutine[Any, Any, R]], + ) -> Callable[P, Coroutine[Any, Any, R]]: + @wraps(func) + async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: + @with_locked_project_from_app( + app, + project_uuid=project_uuid, + status=status, + user_id=user_id, + user_fullname=user_name, + ) + async def _locked_func() -> R: + if notify_users: + await retrieve_and_notify_project_locked_state( + user_id, project_uuid, app + ) + + return await func(*args, **kwargs) + + result = await _locked_func() if notify_users: await retrieve_and_notify_project_locked_state( user_id, project_uuid, app ) - yield - except ProjectLockError: - # someone else has already the lock? - prj_states: ProjectState = await get_project_states_for_user( - user_id, project_uuid, app - ) - log.exception( - "Project [%s] already locked in state '%s'. Please check with support.", - f"{project_uuid=}", - f"{prj_states.locked.status=}", - ) - raise - finally: - if notify_users: - await retrieve_and_notify_project_locked_state(user_id, project_uuid, app) + return result + + return _wrapper + + return _decorator async def get_project_inactivity( diff --git a/services/web/server/tests/unit/with_dbs/02/test_project_lock.py b/services/web/server/tests/unit/with_dbs/02/test_project_lock.py index 71d4602a1f8..9c990a4215e 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_project_lock.py +++ b/services/web/server/tests/unit/with_dbs/02/test_project_lock.py @@ -4,6 +4,8 @@ # pylint: disable=unused-argument # pylint: disable=unused-variable +import asyncio + import pytest import redis.asyncio as aioredis from aiohttp.test_utils import TestClient @@ -13,12 +15,13 @@ from models_library.projects_state import ProjectLocked, ProjectStatus from models_library.users import UserID from pydantic import TypeAdapter +from servicelib.async_utils import cancel_wait_task from simcore_service_webserver.projects.exceptions import ProjectLockError from simcore_service_webserver.projects.lock import ( PROJECT_REDIS_LOCK_KEY, get_project_locked_state, is_project_locked, - lock_project, + with_locked_project_from_app, ) from simcore_service_webserver.users.api import FullNameDict @@ -28,7 +31,7 @@ def project_uuid(faker: Faker) -> ProjectID: return faker.uuid4(cast_to=None) -async def test_lock_project( +async def test_with_locked_project_from_app( client: TestClient, user_id: UserID, project_uuid: ProjectID, @@ -40,13 +43,15 @@ async def test_lock_project( "first_name": faker.first_name(), "last_name": faker.last_name(), } - async with lock_project( + + @with_locked_project_from_app( app=client.app, project_uuid=project_uuid, status=ProjectStatus.EXPORTING, user_id=user_id, user_fullname=user_fullname, - ): + ) + async def _project_locked_fct() -> None: redis_value = await redis_locks_client.get( PROJECT_REDIS_LOCK_KEY.format(project_uuid) ) @@ -58,6 +63,8 @@ async def test_lock_project( status=ProjectStatus.EXPORTING, ) + await _project_locked_fct() + # once the lock is released, the value goes away redis_value = await redis_locks_client.get( PROJECT_REDIS_LOCK_KEY.format(project_uuid) @@ -69,7 +76,6 @@ async def test_lock_already_locked_project_raises( client: TestClient, user_id: UserID, project_uuid: ProjectID, - redis_locks_client: aioredis.Redis, faker: Faker, ): assert client.app @@ -77,23 +83,25 @@ async def test_lock_already_locked_project_raises( "first_name": faker.first_name(), "last_name": faker.last_name(), } - async with lock_project( + + started_event = asyncio.Event() + + @with_locked_project_from_app( app=client.app, project_uuid=project_uuid, status=ProjectStatus.EXPORTING, user_id=user_id, user_fullname=user_name, - ): - # locking again is not permitted - with pytest.raises(ProjectLockError): - async with lock_project( - app=client.app, - project_uuid=project_uuid, - status=ProjectStatus.OPENING, - user_id=user_id, - user_fullname=user_name, - ): - ... + ) + async def _locked_fct() -> None: + started_event.set() + await asyncio.sleep(10) + + task1 = asyncio.create_task(_locked_fct(), name="pytest_task_1") + with pytest.raises(ProjectLockError): + await _locked_fct() + + await cancel_wait_task(task1) async def test_raise_exception_while_locked_release_lock( @@ -108,22 +116,26 @@ async def test_raise_exception_while_locked_release_lock( "first_name": faker.first_name(), "last_name": faker.last_name(), } - with pytest.raises(ValueError): - async with lock_project( - app=client.app, - project_uuid=project_uuid, - status=ProjectStatus.EXPORTING, - user_id=user_id, - user_fullname=user_name, - ): - # here we have the project locked - redis_value = await redis_locks_client.get( - PROJECT_REDIS_LOCK_KEY.format(project_uuid) - ) - assert redis_value - # now raising an exception - msg = "pytest exception" - raise ValueError(msg) + + @with_locked_project_from_app( + app=client.app, + project_uuid=project_uuid, + status=ProjectStatus.EXPORTING, + user_id=user_id, + user_fullname=user_name, + ) + async def _locked_fct() -> None: + # here we have the project locked + redis_value = await redis_locks_client.get( + PROJECT_REDIS_LOCK_KEY.format(project_uuid) + ) + assert redis_value + # now raising an exception + msg = "pytest exception" + raise ValueError(msg) + + with pytest.raises(ValueError, match="pytest exception"): + await _locked_fct() # now the lock shall be released redis_value = await redis_locks_client.get( PROJECT_REDIS_LOCK_KEY.format(project_uuid) @@ -143,15 +155,21 @@ async def test_is_project_locked( "first_name": faker.first_name(), "last_name": faker.last_name(), } - async with lock_project( + + @with_locked_project_from_app( app=client.app, project_uuid=project_uuid, status=ProjectStatus.EXPORTING, user_id=user_id, user_fullname=user_name, - ): + ) + async def _locked_fct() -> None: + assert client.app assert await is_project_locked(client.app, project_uuid) is True + await _locked_fct() + assert await is_project_locked(client.app, project_uuid) is False + @pytest.mark.parametrize( "lock_status", @@ -178,13 +196,16 @@ async def test_get_project_locked_state( "first_name": faker.first_name(), "last_name": faker.last_name(), } - async with lock_project( + + @with_locked_project_from_app( app=client.app, project_uuid=project_uuid, status=lock_status, user_id=user_id, user_fullname=user_name, - ): + ) + async def _locked_fct() -> None: + assert client.app locked_state = await get_project_locked_state(client.app, project_uuid) expected_locked_state = ProjectLocked( value=bool(lock_status not in [ProjectStatus.CLOSED, ProjectStatus.OPENED]), @@ -192,3 +213,5 @@ async def test_get_project_locked_state( status=lock_status, ) assert locked_state == expected_locked_state + + await _locked_fct() diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py index 80f7f4a7bd3..1d489a606ea 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py @@ -36,7 +36,9 @@ from simcore_service_webserver.db.models import UserRole from simcore_service_webserver.projects import _crud_api_delete from simcore_service_webserver.projects.models import ProjectDict -from simcore_service_webserver.projects.projects_api import lock_with_notification +from simcore_service_webserver.projects.projects_api import ( + with_project_locked_notified_state, +) from socketio.exceptions import ConnectionError as SocketConnectionError @@ -228,12 +230,11 @@ async def test_delete_project_while_it_is_locked_raises_error( project_uuid = user_project["uuid"] user_id = logged_user["id"] - async with lock_with_notification( + await with_project_locked_notified_state( app=client.app, project_uuid=project_uuid, status=ProjectStatus.CLOSING, user_id=user_id, user_name={"first_name": "test", "last_name": "test"}, notify_users=False, - ): - await _request_delete_project(client, user_project, expected.conflict) + )(_request_delete_project)(client, user_project, expected.conflict) From 8e6cd324b0604cfa77ec70023a0d6f355f960e74 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 14 Jan 2025 22:16:14 +0100 Subject: [PATCH 06/28] missing wait --- services/web/server/tests/unit/with_dbs/02/test_project_lock.py | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/server/tests/unit/with_dbs/02/test_project_lock.py b/services/web/server/tests/unit/with_dbs/02/test_project_lock.py index 9c990a4215e..54d897d8084 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_project_lock.py +++ b/services/web/server/tests/unit/with_dbs/02/test_project_lock.py @@ -98,6 +98,7 @@ async def _locked_fct() -> None: await asyncio.sleep(10) task1 = asyncio.create_task(_locked_fct(), name="pytest_task_1") + await started_event.wait() with pytest.raises(ProjectLockError): await _locked_fct() From 691eee0ac3f82ecd57e340cef7d430a1beffded6 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 14 Jan 2025 22:24:24 +0100 Subject: [PATCH 07/28] backwards compatibility --- .../src/servicelib/project_lock.py | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/service-library/src/servicelib/project_lock.py b/packages/service-library/src/servicelib/project_lock.py index ab9770de27f..8fb1091d06a 100644 --- a/packages/service-library/src/servicelib/project_lock.py +++ b/packages/service-library/src/servicelib/project_lock.py @@ -9,7 +9,7 @@ from models_library.projects_access import Owner from models_library.projects_state import ProjectLocked, ProjectStatus -from .redis import RedisClientSDK, exclusive +from .redis import CouldNotAcquireLockError, RedisClientSDK, exclusive PROJECT_REDIS_LOCK_KEY: str = "project_lock:{}" PROJECT_LOCK_TIMEOUT: Final[datetime.timedelta] = datetime.timedelta(seconds=10) @@ -33,18 +33,24 @@ def with_locked_project( def _decorator( func: Callable[P, Coroutine[Any, Any, R]], ) -> Callable[P, Coroutine[Any, Any, R]]: - @exclusive( - redis_client, - lock_key=PROJECT_REDIS_LOCK_KEY.format(project_uuid), - lock_value=ProjectLocked( - value=True, - owner=owner, - status=status, - ).model_dump_json(), - ) @functools.wraps(func) async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: - return await func(*args, **kwargs) + @exclusive( + redis_client, + lock_key=PROJECT_REDIS_LOCK_KEY.format(project_uuid), + lock_value=ProjectLocked( + value=True, + owner=owner, + status=status, + ).model_dump_json(), + ) + async def _exclusive_func(*args, **kwargs) -> R: + return await func(*args, **kwargs) + + try: + return await _exclusive_func(*args, **kwargs) + except CouldNotAcquireLockError as e: + raise ProjectLockError from e return _wrapper From 4a01d0478bc31ea9ab6dab32fb893f8142eed2f6 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Jan 2025 18:00:34 +0100 Subject: [PATCH 08/28] rename --- packages/service-library/src/servicelib/project_lock.py | 2 +- .../simcore_service_efs_guardian/services/background_tasks.py | 4 ++-- .../web/server/src/simcore_service_webserver/projects/lock.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/service-library/src/servicelib/project_lock.py b/packages/service-library/src/servicelib/project_lock.py index 8fb1091d06a..ad81810523b 100644 --- a/packages/service-library/src/servicelib/project_lock.py +++ b/packages/service-library/src/servicelib/project_lock.py @@ -21,7 +21,7 @@ R = TypeVar("R") -def with_locked_project( +def with_project_locked( redis_client: RedisClientSDK | Callable[..., RedisClientSDK], *, project_uuid: str | ProjectID, diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py index e49e5a31348..a008671303f 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py @@ -5,7 +5,7 @@ from models_library.projects import ProjectID from models_library.projects_state import ProjectStatus from servicelib.logging_utils import log_context -from servicelib.project_lock import with_locked_project +from servicelib.project_lock import with_project_locked from simcore_postgres_database.utils_projects import ( DBProjectNotFoundError, ProjectsRepo, @@ -22,7 +22,7 @@ async def _remove_data_with_lock(app: FastAPI, project_id: ProjectID) -> None: # Decorate a new function that will call the necessary coroutine efs_manager: EfsManager = app.state.efs_manager - @with_locked_project( + @with_project_locked( get_redis_lock_client(app), project_uuid=project_id, status=ProjectStatus.MAINTAINING, diff --git a/services/web/server/src/simcore_service_webserver/projects/lock.py b/services/web/server/src/simcore_service_webserver/projects/lock.py index 81e4987c918..eecdf785afd 100644 --- a/services/web/server/src/simcore_service_webserver/projects/lock.py +++ b/services/web/server/src/simcore_service_webserver/projects/lock.py @@ -6,7 +6,7 @@ from models_library.projects import ProjectID from models_library.projects_access import Owner from models_library.projects_state import ProjectLocked, ProjectStatus -from servicelib.project_lock import PROJECT_REDIS_LOCK_KEY, with_locked_project +from servicelib.project_lock import PROJECT_REDIS_LOCK_KEY, with_project_locked from ..redis import get_redis_lock_manager_client, get_redis_lock_manager_client_sdk from ..users.api import FullNameDict @@ -28,7 +28,7 @@ def with_locked_project_from_app( def _decorator( func: Callable[P, Coroutine[Any, Any, R]], ) -> Callable[P, Coroutine[Any, Any, R]]: - @with_locked_project( + @with_project_locked( get_redis_lock_manager_client_sdk(app), project_uuid=project_uuid, status=status, From ae4bdb6613cd14d38a9a06c1f60032209adde76b Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Jan 2025 18:03:08 +0100 Subject: [PATCH 09/28] rename --- .../services/background_tasks.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py index a008671303f..f74cd23cd50 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py @@ -18,8 +18,7 @@ _logger = logging.getLogger(__name__) -async def _remove_data_with_lock(app: FastAPI, project_id: ProjectID) -> None: - # Decorate a new function that will call the necessary coroutine +async def _lock_project_and_remove_data(app: FastAPI, project_id: ProjectID) -> None: efs_manager: EfsManager = app.state.efs_manager @with_project_locked( @@ -28,10 +27,8 @@ async def _remove_data_with_lock(app: FastAPI, project_id: ProjectID) -> None: status=ProjectStatus.MAINTAINING, ) async def _remove(): - # Call the actual coroutine function await efs_manager.remove_project_efs_data(project_id) - # Execute the decorated function await _remove() @@ -74,4 +71,4 @@ async def removal_policy_task(app: FastAPI) -> None: logging.INFO, msg=f"Removing data for project {project_id} started, project last change date {_project_last_change_date}, efs removal policy task age limit timedelta {app_settings.EFS_REMOVAL_POLICY_TASK_AGE_LIMIT_TIMEDELTA}", ): - await _remove_data_with_lock(app, project_id) + await _lock_project_and_remove_data(app, project_id) From 9740b2f1e2e0470d1137da8bb8bcee7c09a0af1d Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Jan 2025 18:24:29 +0100 Subject: [PATCH 10/28] refactoring --- .../src/servicelib/project_lock.py | 23 +++++ .../projects/_crud_handlers.py | 13 +-- .../projects/lock.py | 69 -------------- .../projects/projects_api.py | 23 +++-- .../unit/with_dbs/02/test_project_lock.py | 90 ++++++++++--------- 5 files changed, 90 insertions(+), 128 deletions(-) delete mode 100644 services/web/server/src/simcore_service_webserver/projects/lock.py diff --git a/packages/service-library/src/servicelib/project_lock.py b/packages/service-library/src/servicelib/project_lock.py index ad81810523b..6745a3b7fb2 100644 --- a/packages/service-library/src/servicelib/project_lock.py +++ b/packages/service-library/src/servicelib/project_lock.py @@ -55,3 +55,26 @@ async def _exclusive_func(*args, **kwargs) -> R: return _wrapper return _decorator + + +async def is_project_locked( + redis_client: RedisClientSDK, project_uuid: str | ProjectID +) -> bool: + redis_lock = redis_client.create_lock(PROJECT_REDIS_LOCK_KEY.format(project_uuid)) + return await redis_lock.locked() + + +async def get_project_locked_state( + redis_client: RedisClientSDK, project_uuid: str | ProjectID +) -> ProjectLocked | None: + """ + Returns: + ProjectLocked object if the project project_uuid is locked or None otherwise + """ + if await is_project_locked(redis_client, project_uuid=project_uuid) and ( + lock_value := await redis_client.redis.get( + PROJECT_REDIS_LOCK_KEY.format(project_uuid) + ) + ): + return ProjectLocked.model_validate_json(lock_value) + return None diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index 2ea0d84b808..d809e153965 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -1,4 +1,4 @@ -""" Handlers for STANDARD methods on /projects colletions +"""Handlers for STANDARD methods on /projects colletions Standard methods or CRUD that states for Create+Read(Get&List)+Update+Delete @@ -36,12 +36,14 @@ X_SIMCORE_USER_AGENT, ) from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON +from servicelib.project_lock import get_project_locked_state from servicelib.rest_constants import RESPONSE_MODEL_POLICY from .._meta import API_VTAG as VTAG from ..catalog.client import get_services_for_user_in_product from ..folders.errors import FolderAccessForbiddenError, FolderNotFoundError from ..login.decorators import login_required +from ..redis import get_redis_lock_manager_client_sdk from ..resource_manager.user_sessions import PROJECT_ID_KEY, managed_resource from ..security.api import check_user_permission from ..security.decorators import permission_required @@ -65,7 +67,6 @@ ProjectOwnerNotFoundInTheProjectAccessRightsError, WrongTagIdsInQueryError, ) -from .lock import get_project_locked_state from .models import ProjectDict from .utils import get_project_unavailable_services, project_uses_available_services @@ -139,7 +140,8 @@ async def create_project(request: web.Request): project_create: ( ProjectCreateNew | ProjectCopyOverride | EmptyModel ) = await parse_request_body_as( - ProjectCreateNew | ProjectCopyOverride | EmptyModel, request # type: ignore[arg-type] # from pydantic v2 --> https://github.com/pydantic/pydantic/discussions/4950 + ProjectCreateNew | ProjectCopyOverride | EmptyModel, + request, # type: ignore[arg-type] # from pydantic v2 --> https://github.com/pydantic/pydantic/discussions/4950 ) predefined_project = ( project_create.model_dump( @@ -457,7 +459,7 @@ async def delete_project(request: web.Request): ) if project_users: other_user_names = { - await get_user_fullname(request.app, user_id=uid) + f"{await get_user_fullname(request.app, user_id=uid)}" for uid in project_users } raise web.HTTPForbidden( @@ -467,7 +469,8 @@ async def delete_project(request: web.Request): project_locked_state: ProjectLocked | None if project_locked_state := await get_project_locked_state( - app=request.app, project_uuid=path_params.project_id + get_redis_lock_manager_client_sdk(request.app), + project_uuid=path_params.project_id, ): raise web.HTTPConflict( reason=f"Project {path_params.project_id} is locked: {project_locked_state=}" diff --git a/services/web/server/src/simcore_service_webserver/projects/lock.py b/services/web/server/src/simcore_service_webserver/projects/lock.py deleted file mode 100644 index eecdf785afd..00000000000 --- a/services/web/server/src/simcore_service_webserver/projects/lock.py +++ /dev/null @@ -1,69 +0,0 @@ -from collections.abc import Callable, Coroutine -from functools import wraps -from typing import Any, ParamSpec, TypeVar - -from aiohttp import web -from models_library.projects import ProjectID -from models_library.projects_access import Owner -from models_library.projects_state import ProjectLocked, ProjectStatus -from servicelib.project_lock import PROJECT_REDIS_LOCK_KEY, with_project_locked - -from ..redis import get_redis_lock_manager_client, get_redis_lock_manager_client_sdk -from ..users.api import FullNameDict - -P = ParamSpec("P") -R = TypeVar("R") - - -def with_locked_project_from_app( - app: web.Application, - *, - project_uuid: str | ProjectID, - status: ProjectStatus, - user_id: int, - user_fullname: FullNameDict, -) -> Callable[ - [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] -]: - def _decorator( - func: Callable[P, Coroutine[Any, Any, R]], - ) -> Callable[P, Coroutine[Any, Any, R]]: - @with_project_locked( - get_redis_lock_manager_client_sdk(app), - project_uuid=project_uuid, - status=status, - owner=Owner(user_id=user_id, **user_fullname), - ) - @wraps(func) - async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: - return await func(*args, **kwargs) - - return _wrapper - - return _decorator - - -async def is_project_locked( - app: web.Application, project_uuid: str | ProjectID -) -> bool: - redis_lock = get_redis_lock_manager_client(app).lock( - PROJECT_REDIS_LOCK_KEY.format(project_uuid) - ) - return await redis_lock.locked() - - -async def get_project_locked_state( - app: web.Application, project_uuid: str | ProjectID -) -> ProjectLocked | None: - """ - Returns: - ProjectLocked object if the project project_uuid is locked or None otherwise - """ - if await is_project_locked(app, project_uuid): - redis_locks_client = get_redis_lock_manager_client(app) - - if lock_value := await redis_locks_client.get( - PROJECT_REDIS_LOCK_KEY.format(project_uuid) - ): - return ProjectLocked.model_validate_json(lock_value) - return None diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index c3c3b2ca6f5..8fb87e5b7f9 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -72,6 +72,11 @@ X_SIMCORE_USER_AGENT, ) from servicelib.logging_utils import get_log_record_extra, log_context +from servicelib.project_lock import ( + get_project_locked_state, + is_project_locked, + with_project_locked, +) from servicelib.rabbitmq import RemoteMethodNotRegisteredError, RPCServerError from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc from servicelib.rabbitmq.rpc_interfaces.clusters_keeper.ec2_instances import ( @@ -145,11 +150,6 @@ ProjectStartsTooManyDynamicNodesError, ProjectTooManyProjectOpenedError, ) -from .lock import ( - get_project_locked_state, - is_project_locked, - with_locked_project_from_app, -) from .models import ProjectDict, ProjectPatchExtended from .settings import ProjectsSettings, get_plugin_settings from .utils import extract_dns_without_default_port @@ -286,7 +286,7 @@ async def patch_project( "delete": True, } user: dict = await get_user(app, project_db.prj_owner) - _prj_owner_primary_group = f'{user["primary_gid"]}' + _prj_owner_primary_group = f"{user['primary_gid']}" if _prj_owner_primary_group not in new_prj_access_rights: raise ProjectOwnerNotFoundInTheProjectAccessRightsError if new_prj_access_rights[_prj_owner_primary_group] != _prj_required_permissions: @@ -1154,7 +1154,7 @@ async def _trigger_connected_service_retrieve( app: web.Application, project: dict, updated_node_uuid: str, changed_keys: list[str] ) -> None: project_id = project["uuid"] - if await is_project_locked(app, project_id): + if await is_project_locked(get_redis_lock_manager_client_sdk(app), project_id): # NOTE: we log warn since this function is fire&forget and raise an exception would not be anybody to handle it log.warning( "Skipping service retrieval because project with %s is currently locked." @@ -1409,7 +1409,7 @@ async def _get_project_lock_state( f"{user_id=}", ) prj_locked_state: ProjectLocked | None = await get_project_locked_state( - app, project_uuid + get_redis_lock_manager_client_sdk(app), project_uuid ) if prj_locked_state: log.debug( @@ -1880,12 +1880,11 @@ def _decorator( ) -> Callable[P, Coroutine[Any, Any, R]]: @wraps(func) async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: - @with_locked_project_from_app( - app, + @with_project_locked( + get_redis_lock_manager_client_sdk(app), project_uuid=project_uuid, status=status, - user_id=user_id, - user_fullname=user_name, + owner=Owner(user_id=user_id, **user_name), ) async def _locked_func() -> R: if notify_users: diff --git a/services/web/server/tests/unit/with_dbs/02/test_project_lock.py b/services/web/server/tests/unit/with_dbs/02/test_project_lock.py index 54d897d8084..6213314a6a4 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_project_lock.py +++ b/services/web/server/tests/unit/with_dbs/02/test_project_lock.py @@ -5,6 +5,8 @@ # pylint: disable=unused-variable import asyncio +from typing import cast +from uuid import UUID import pytest import redis.asyncio as aioredis @@ -16,40 +18,46 @@ from models_library.users import UserID from pydantic import TypeAdapter from servicelib.async_utils import cancel_wait_task -from simcore_service_webserver.projects.exceptions import ProjectLockError -from simcore_service_webserver.projects.lock import ( - PROJECT_REDIS_LOCK_KEY, +from servicelib.project_lock import ( get_project_locked_state, is_project_locked, - with_locked_project_from_app, + with_project_locked, ) +from servicelib.redis._client import RedisClientSDK +from simcore_service_webserver.projects.exceptions import ProjectLockError +from simcore_service_webserver.projects.lock import PROJECT_REDIS_LOCK_KEY +from simcore_service_webserver.redis import get_redis_lock_manager_client_sdk from simcore_service_webserver.users.api import FullNameDict @pytest.fixture() def project_uuid(faker: Faker) -> ProjectID: - return faker.uuid4(cast_to=None) + return cast(UUID, faker.uuid4(cast_to=None)) + + +@pytest.fixture() +def redis_client_from_app(client: TestClient) -> RedisClientSDK: + assert client.app + return get_redis_lock_manager_client_sdk(client.app) async def test_with_locked_project_from_app( - client: TestClient, + redis_client_from_app: RedisClientSDK, user_id: UserID, project_uuid: ProjectID, redis_locks_client: aioredis.Redis, faker: Faker, ): - assert client.app user_fullname: FullNameDict = { "first_name": faker.first_name(), "last_name": faker.last_name(), } - @with_locked_project_from_app( - app=client.app, + @with_project_locked( + redis_client_from_app, project_uuid=project_uuid, status=ProjectStatus.EXPORTING, - user_id=user_id, - user_fullname=user_fullname, + owner=Owner(user_id=user_id, **user_fullname), ) async def _project_locked_fct() -> None: redis_value = await redis_locks_client.get( @@ -73,25 +81,23 @@ async def _project_locked_fct() -> None: async def test_lock_already_locked_project_raises( - client: TestClient, + redis_client_from_app: RedisClientSDK, user_id: UserID, project_uuid: ProjectID, faker: Faker, ): - assert client.app - user_name: FullNameDict = { + user_fullname: FullNameDict = { "first_name": faker.first_name(), "last_name": faker.last_name(), } started_event = asyncio.Event() - @with_locked_project_from_app( - app=client.app, + @with_project_locked( + redis_client_from_app, project_uuid=project_uuid, status=ProjectStatus.EXPORTING, - user_id=user_id, - user_fullname=user_name, + owner=Owner(user_id=user_id, **user_fullname), ) async def _locked_fct() -> None: started_event.set() @@ -106,24 +112,22 @@ async def _locked_fct() -> None: async def test_raise_exception_while_locked_release_lock( - client: TestClient, + redis_client_from_app: RedisClientSDK, user_id: UserID, project_uuid: ProjectID, redis_locks_client: aioredis.Redis, faker: Faker, ): - assert client.app - user_name: FullNameDict = { + user_fullname: FullNameDict = { "first_name": faker.first_name(), "last_name": faker.last_name(), } - @with_locked_project_from_app( - app=client.app, + @with_project_locked( + redis_client_from_app, project_uuid=project_uuid, status=ProjectStatus.EXPORTING, - user_id=user_id, - user_fullname=user_name, + owner=Owner(user_id=user_id, **user_fullname), ) async def _locked_fct() -> None: # here we have the project locked @@ -145,31 +149,31 @@ async def _locked_fct() -> None: async def test_is_project_locked( + redis_client_from_app: RedisClientSDK, client: TestClient, user_id: UserID, project_uuid: ProjectID, faker: Faker, ): assert client.app - assert await is_project_locked(client.app, project_uuid) is False - user_name: FullNameDict = { + assert await is_project_locked(redis_client_from_app, project_uuid) is False + user_fullname: FullNameDict = { "first_name": faker.first_name(), "last_name": faker.last_name(), } - @with_locked_project_from_app( - app=client.app, + @with_project_locked( + redis_client_from_app, project_uuid=project_uuid, status=ProjectStatus.EXPORTING, - user_id=user_id, - user_fullname=user_name, + owner=Owner(user_id=user_id, **user_fullname), ) async def _locked_fct() -> None: assert client.app - assert await is_project_locked(client.app, project_uuid) is True + assert await is_project_locked(redis_client_from_app, project_uuid) is True await _locked_fct() - assert await is_project_locked(client.app, project_uuid) is False + assert await is_project_locked(redis_client_from_app, project_uuid) is False @pytest.mark.parametrize( @@ -182,6 +186,7 @@ async def _locked_fct() -> None: ], ) async def test_get_project_locked_state( + redis_client_from_app: RedisClientSDK, client: TestClient, user_id: UserID, project_uuid: ProjectID, @@ -190,27 +195,28 @@ async def test_get_project_locked_state( ): assert client.app # no lock - assert await get_project_locked_state(client.app, project_uuid) is None + assert await get_project_locked_state(redis_client_from_app, project_uuid) is None - assert await is_project_locked(client.app, project_uuid) is False - user_name: FullNameDict = { + assert await is_project_locked(redis_client_from_app, project_uuid) is False + user_fullname: FullNameDict = { "first_name": faker.first_name(), "last_name": faker.last_name(), } - @with_locked_project_from_app( - app=client.app, + @with_project_locked( + redis_client_from_app, project_uuid=project_uuid, status=lock_status, - user_id=user_id, - user_fullname=user_name, + owner=Owner(user_id=user_id, **user_fullname), ) async def _locked_fct() -> None: assert client.app - locked_state = await get_project_locked_state(client.app, project_uuid) + locked_state = await get_project_locked_state( + redis_client_from_app, project_uuid + ) expected_locked_state = ProjectLocked( value=bool(lock_status not in [ProjectStatus.CLOSED, ProjectStatus.OPENED]), - owner=Owner(user_id=user_id, **user_name), + owner=Owner(user_id=user_id, **user_fullname), status=lock_status, ) assert locked_state == expected_locked_state From 03cb7bd143a0a92b8cbee441ad09db4bfd5a7e39 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Jan 2025 21:51:24 +0100 Subject: [PATCH 11/28] moved project_lock in servicelib --- .../src/servicelib/redis/__init__.py | 12 + .../_project_lock.py} | 6 +- .../tests/redis/test_decorators.py | 2 +- .../tests/redis/test_project_lock.py | 131 ++++++++++ .../tests/test_project_lock.py | 1 - .../services/background_tasks.py | 2 +- .../projects/_crud_handlers.py | 2 +- .../projects/exceptions.py | 2 +- .../projects/projects_api.py | 10 +- .../unit/with_dbs/02/test_project_lock.py | 224 ------------------ 10 files changed, 154 insertions(+), 238 deletions(-) rename packages/service-library/src/servicelib/{project_lock.py => redis/_project_lock.py} (91%) create mode 100644 packages/service-library/tests/redis/test_project_lock.py delete mode 100644 packages/service-library/tests/test_project_lock.py delete mode 100644 services/web/server/tests/unit/with_dbs/02/test_project_lock.py diff --git a/packages/service-library/src/servicelib/redis/__init__.py b/packages/service-library/src/servicelib/redis/__init__.py index 8d78d47ece5..513d120fbd3 100644 --- a/packages/service-library/src/servicelib/redis/__init__.py +++ b/packages/service-library/src/servicelib/redis/__init__.py @@ -7,17 +7,29 @@ LockLostError, ) from ._models import RedisManagerDBConfig +from ._project_lock import ( + PROJECT_REDIS_LOCK_KEY, + ProjectLockError, + get_project_locked_state, + is_project_locked, + with_project_locked, +) from ._utils import handle_redis_returns_union_types __all__: tuple[str, ...] = ( "CouldNotAcquireLockError", "CouldNotConnectToRedisError", "exclusive", + "get_project_locked_state", "handle_redis_returns_union_types", + "is_project_locked", "LockLostError", + "PROJECT_REDIS_LOCK_KEY", + "ProjectLockError", "RedisClientSDK", "RedisClientsManager", "RedisManagerDBConfig", + "with_project_locked", ) # nopycln: file diff --git a/packages/service-library/src/servicelib/project_lock.py b/packages/service-library/src/servicelib/redis/_project_lock.py similarity index 91% rename from packages/service-library/src/servicelib/project_lock.py rename to packages/service-library/src/servicelib/redis/_project_lock.py index 6745a3b7fb2..651e24c784a 100644 --- a/packages/service-library/src/servicelib/project_lock.py +++ b/packages/service-library/src/servicelib/redis/_project_lock.py @@ -1,4 +1,3 @@ -import datetime import functools from collections.abc import Callable, Coroutine from typing import Any, Final, ParamSpec, TypeAlias, TypeVar @@ -9,10 +8,9 @@ from models_library.projects_access import Owner from models_library.projects_state import ProjectLocked, ProjectStatus -from .redis import CouldNotAcquireLockError, RedisClientSDK, exclusive +from . import CouldNotAcquireLockError, RedisClientSDK, exclusive -PROJECT_REDIS_LOCK_KEY: str = "project_lock:{}" -PROJECT_LOCK_TIMEOUT: Final[datetime.timedelta] = datetime.timedelta(seconds=10) +PROJECT_REDIS_LOCK_KEY: Final[str] = "project_lock:{}" ProjectLockError: TypeAlias = redis.exceptions.LockError diff --git a/packages/service-library/tests/redis/test_decorators.py b/packages/service-library/tests/redis/test_decorators.py index 643cfef99d8..e4ca9d51463 100644 --- a/packages/service-library/tests/redis/test_decorators.py +++ b/packages/service-library/tests/redis/test_decorators.py @@ -75,7 +75,7 @@ async def _(): pass -async def test_exclusive_decorator( +async def test_exclusive_decorator_runs_original_method( redis_client_sdk: RedisClientSDK, lock_name: str, sleep_duration: float, diff --git a/packages/service-library/tests/redis/test_project_lock.py b/packages/service-library/tests/redis/test_project_lock.py new file mode 100644 index 00000000000..e22e8ae9b3e --- /dev/null +++ b/packages/service-library/tests/redis/test_project_lock.py @@ -0,0 +1,131 @@ +# pylint: disable=no-value-for-parameter +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable + +import asyncio +from typing import cast +from uuid import UUID + +import pytest +from faker import Faker +from models_library.projects import ProjectID +from models_library.projects_access import Owner +from models_library.projects_state import ProjectLocked, ProjectStatus +from servicelib.async_utils import cancel_wait_task +from servicelib.redis import ( + PROJECT_REDIS_LOCK_KEY, + ProjectLockError, + RedisClientSDK, + get_project_locked_state, + is_project_locked, + with_project_locked, +) + +pytest_simcore_core_services_selection = [ + "redis", +] +pytest_simcore_ops_services_selection = [ + "redis-commander", +] + + +@pytest.fixture() +def project_uuid(faker: Faker) -> ProjectID: + return cast(UUID, faker.uuid4(cast_to=None)) + + +assert "json_schema_extra" in Owner.model_config +assert isinstance(Owner.model_config["json_schema_extra"], dict) +assert isinstance(Owner.model_config["json_schema_extra"]["examples"], list) + + +@pytest.fixture(params=Owner.model_config["json_schema_extra"]["examples"]) +def owner(request: pytest.FixtureRequest) -> Owner: + return Owner(**request.param) + + +@pytest.mark.parametrize( + "project_status", + [ + ProjectStatus.CLOSING, + ProjectStatus.CLONING, + ProjectStatus.EXPORTING, + ProjectStatus.OPENING, + ProjectStatus.MAINTAINING, + ], +) +async def test_with_project_locked( + redis_client_sdk: RedisClientSDK, + project_uuid: ProjectID, + owner: Owner, + project_status: ProjectStatus, +): + @with_project_locked( + redis_client_sdk, + project_uuid=project_uuid, + status=project_status, + owner=owner, + ) + async def _locked_fct() -> None: + assert await is_project_locked(redis_client_sdk, project_uuid) is True + locked_state = await get_project_locked_state(redis_client_sdk, project_uuid) + assert locked_state is not None + assert locked_state == ProjectLocked( + value=True, + owner=owner, + status=project_status, + ) + # check lock name formatting is correct + redis_lock = await redis_client_sdk.redis.get( + PROJECT_REDIS_LOCK_KEY.format(project_uuid) + ) + assert redis_lock + assert ProjectLocked.model_validate_json(redis_lock) == ProjectLocked( + value=True, + owner=owner, + status=project_status, + ) + + assert await get_project_locked_state(redis_client_sdk, project_uuid) is None + assert await is_project_locked(redis_client_sdk, project_uuid) is False + await _locked_fct() + assert await is_project_locked(redis_client_sdk, project_uuid) is False + assert await get_project_locked_state(redis_client_sdk, project_uuid) is None + + +@pytest.mark.parametrize( + "project_status", + [ + ProjectStatus.CLOSING, + ProjectStatus.CLONING, + ProjectStatus.EXPORTING, + ProjectStatus.OPENING, + ProjectStatus.MAINTAINING, + ], +) +async def test_lock_already_locked_project_raises( + redis_client_sdk: RedisClientSDK, + project_uuid: ProjectID, + owner: Owner, + project_status: ProjectStatus, +): + started_event = asyncio.Event() + + @with_project_locked( + redis_client_sdk, + project_uuid=project_uuid, + status=project_status, + owner=owner, + ) + async def _locked_fct() -> None: + started_event.set() + await asyncio.sleep(10) + + task1 = asyncio.create_task(_locked_fct(), name="pytest_task_1") + await started_event.wait() + with pytest.raises(ProjectLockError): + await _locked_fct() + + await cancel_wait_task(task1) diff --git a/packages/service-library/tests/test_project_lock.py b/packages/service-library/tests/test_project_lock.py deleted file mode 100644 index 386c14be3fb..00000000000 --- a/packages/service-library/tests/test_project_lock.py +++ /dev/null @@ -1 +0,0 @@ -# NOTE: Tested in osparc-simcore/services/web/server/tests/unit/with_dbs/02/test_project_lock.py diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py index f74cd23cd50..fca4a2fafde 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py @@ -5,7 +5,7 @@ from models_library.projects import ProjectID from models_library.projects_state import ProjectStatus from servicelib.logging_utils import log_context -from servicelib.project_lock import with_project_locked +from servicelib.redis import with_project_locked from simcore_postgres_database.utils_projects import ( DBProjectNotFoundError, ProjectsRepo, diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index d809e153965..53e186d399b 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -36,7 +36,7 @@ X_SIMCORE_USER_AGENT, ) from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON -from servicelib.project_lock import get_project_locked_state +from servicelib.redis import get_project_locked_state from servicelib.rest_constants import RESPONSE_MODEL_POLICY from .._meta import API_VTAG as VTAG diff --git a/services/web/server/src/simcore_service_webserver/projects/exceptions.py b/services/web/server/src/simcore_service_webserver/projects/exceptions.py index cfa3506dada..66197a2f9df 100644 --- a/services/web/server/src/simcore_service_webserver/projects/exceptions.py +++ b/services/web/server/src/simcore_service_webserver/projects/exceptions.py @@ -5,7 +5,7 @@ from models_library.projects import ProjectID from models_library.users import UserID -from servicelib.project_lock import ProjectLockError +from servicelib.redis import ProjectLockError from ..errors import WebServerBaseError diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index 8fb87e5b7f9..a239a4fde78 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -72,11 +72,6 @@ X_SIMCORE_USER_AGENT, ) from servicelib.logging_utils import get_log_record_extra, log_context -from servicelib.project_lock import ( - get_project_locked_state, - is_project_locked, - with_project_locked, -) from servicelib.rabbitmq import RemoteMethodNotRegisteredError, RPCServerError from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc from servicelib.rabbitmq.rpc_interfaces.clusters_keeper.ec2_instances import ( @@ -86,6 +81,11 @@ ServiceWaitingForManualInterventionError, ServiceWasNotFoundError, ) +from servicelib.redis import ( + get_project_locked_state, + is_project_locked, + with_project_locked, +) from servicelib.redis._decorators import exclusive from servicelib.utils import fire_and_forget_task, logged_gather from simcore_postgres_database.models.users import UserRole diff --git a/services/web/server/tests/unit/with_dbs/02/test_project_lock.py b/services/web/server/tests/unit/with_dbs/02/test_project_lock.py deleted file mode 100644 index 6213314a6a4..00000000000 --- a/services/web/server/tests/unit/with_dbs/02/test_project_lock.py +++ /dev/null @@ -1,224 +0,0 @@ -# pylint: disable=no-value-for-parameter -# pylint: disable=protected-access -# pylint: disable=redefined-outer-name -# pylint: disable=unused-argument -# pylint: disable=unused-variable - -import asyncio -from typing import cast -from uuid import UUID - -import pytest -import redis.asyncio as aioredis -from aiohttp.test_utils import TestClient -from faker import Faker -from models_library.projects import ProjectID -from models_library.projects_access import Owner -from models_library.projects_state import ProjectLocked, ProjectStatus -from models_library.users import UserID -from pydantic import TypeAdapter -from servicelib.async_utils import cancel_wait_task -from servicelib.project_lock import ( - get_project_locked_state, - is_project_locked, - with_project_locked, -) -from servicelib.redis._client import RedisClientSDK -from simcore_service_webserver.projects.exceptions import ProjectLockError -from simcore_service_webserver.projects.lock import PROJECT_REDIS_LOCK_KEY -from simcore_service_webserver.redis import get_redis_lock_manager_client_sdk -from simcore_service_webserver.users.api import FullNameDict - - -@pytest.fixture() -def project_uuid(faker: Faker) -> ProjectID: - return cast(UUID, faker.uuid4(cast_to=None)) - - -@pytest.fixture() -def redis_client_from_app(client: TestClient) -> RedisClientSDK: - assert client.app - return get_redis_lock_manager_client_sdk(client.app) - - -async def test_with_locked_project_from_app( - redis_client_from_app: RedisClientSDK, - user_id: UserID, - project_uuid: ProjectID, - redis_locks_client: aioredis.Redis, - faker: Faker, -): - user_fullname: FullNameDict = { - "first_name": faker.first_name(), - "last_name": faker.last_name(), - } - - @with_project_locked( - redis_client_from_app, - project_uuid=project_uuid, - status=ProjectStatus.EXPORTING, - owner=Owner(user_id=user_id, **user_fullname), - ) - async def _project_locked_fct() -> None: - redis_value = await redis_locks_client.get( - PROJECT_REDIS_LOCK_KEY.format(project_uuid) - ) - assert redis_value - lock_value = TypeAdapter(ProjectLocked).validate_json(redis_value) - assert lock_value == ProjectLocked( - value=True, - owner=Owner(user_id=user_id, **user_fullname), - status=ProjectStatus.EXPORTING, - ) - - await _project_locked_fct() - - # once the lock is released, the value goes away - redis_value = await redis_locks_client.get( - PROJECT_REDIS_LOCK_KEY.format(project_uuid) - ) - assert not redis_value - - -async def test_lock_already_locked_project_raises( - redis_client_from_app: RedisClientSDK, - user_id: UserID, - project_uuid: ProjectID, - faker: Faker, -): - user_fullname: FullNameDict = { - "first_name": faker.first_name(), - "last_name": faker.last_name(), - } - - started_event = asyncio.Event() - - @with_project_locked( - redis_client_from_app, - project_uuid=project_uuid, - status=ProjectStatus.EXPORTING, - owner=Owner(user_id=user_id, **user_fullname), - ) - async def _locked_fct() -> None: - started_event.set() - await asyncio.sleep(10) - - task1 = asyncio.create_task(_locked_fct(), name="pytest_task_1") - await started_event.wait() - with pytest.raises(ProjectLockError): - await _locked_fct() - - await cancel_wait_task(task1) - - -async def test_raise_exception_while_locked_release_lock( - redis_client_from_app: RedisClientSDK, - user_id: UserID, - project_uuid: ProjectID, - redis_locks_client: aioredis.Redis, - faker: Faker, -): - user_fullname: FullNameDict = { - "first_name": faker.first_name(), - "last_name": faker.last_name(), - } - - @with_project_locked( - redis_client_from_app, - project_uuid=project_uuid, - status=ProjectStatus.EXPORTING, - owner=Owner(user_id=user_id, **user_fullname), - ) - async def _locked_fct() -> None: - # here we have the project locked - redis_value = await redis_locks_client.get( - PROJECT_REDIS_LOCK_KEY.format(project_uuid) - ) - assert redis_value - # now raising an exception - msg = "pytest exception" - raise ValueError(msg) - - with pytest.raises(ValueError, match="pytest exception"): - await _locked_fct() - # now the lock shall be released - redis_value = await redis_locks_client.get( - PROJECT_REDIS_LOCK_KEY.format(project_uuid) - ) - assert not redis_value - - -async def test_is_project_locked( - redis_client_from_app: RedisClientSDK, - client: TestClient, - user_id: UserID, - project_uuid: ProjectID, - faker: Faker, -): - assert client.app - assert await is_project_locked(redis_client_from_app, project_uuid) is False - user_fullname: FullNameDict = { - "first_name": faker.first_name(), - "last_name": faker.last_name(), - } - - @with_project_locked( - redis_client_from_app, - project_uuid=project_uuid, - status=ProjectStatus.EXPORTING, - owner=Owner(user_id=user_id, **user_fullname), - ) - async def _locked_fct() -> None: - assert client.app - assert await is_project_locked(redis_client_from_app, project_uuid) is True - - await _locked_fct() - assert await is_project_locked(redis_client_from_app, project_uuid) is False - - -@pytest.mark.parametrize( - "lock_status", - [ - ProjectStatus.CLOSING, - ProjectStatus.CLONING, - ProjectStatus.EXPORTING, - ProjectStatus.OPENING, - ], -) -async def test_get_project_locked_state( - redis_client_from_app: RedisClientSDK, - client: TestClient, - user_id: UserID, - project_uuid: ProjectID, - faker: Faker, - lock_status: ProjectStatus, -): - assert client.app - # no lock - assert await get_project_locked_state(redis_client_from_app, project_uuid) is None - - assert await is_project_locked(redis_client_from_app, project_uuid) is False - user_fullname: FullNameDict = { - "first_name": faker.first_name(), - "last_name": faker.last_name(), - } - - @with_project_locked( - redis_client_from_app, - project_uuid=project_uuid, - status=lock_status, - owner=Owner(user_id=user_id, **user_fullname), - ) - async def _locked_fct() -> None: - assert client.app - locked_state = await get_project_locked_state( - redis_client_from_app, project_uuid - ) - expected_locked_state = ProjectLocked( - value=bool(lock_status not in [ProjectStatus.CLOSED, ProjectStatus.OPENED]), - owner=Owner(user_id=user_id, **user_fullname), - status=lock_status, - ) - assert locked_state == expected_locked_state - - await _locked_fct() From abd060cfd4e66c9c2bacab9913313c7e5d8bf246 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Jan 2025 21:53:54 +0100 Subject: [PATCH 12/28] refactor --- packages/service-library/src/servicelib/redis/__init__.py | 2 +- packages/service-library/src/servicelib/redis/_errors.py | 6 ++++++ .../service-library/src/servicelib/redis/_project_lock.py | 7 ++----- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/service-library/src/servicelib/redis/__init__.py b/packages/service-library/src/servicelib/redis/__init__.py index 513d120fbd3..5f7fb2b4f31 100644 --- a/packages/service-library/src/servicelib/redis/__init__.py +++ b/packages/service-library/src/servicelib/redis/__init__.py @@ -5,11 +5,11 @@ CouldNotAcquireLockError, CouldNotConnectToRedisError, LockLostError, + ProjectLockError, ) from ._models import RedisManagerDBConfig from ._project_lock import ( PROJECT_REDIS_LOCK_KEY, - ProjectLockError, get_project_locked_state, is_project_locked, with_project_locked, diff --git a/packages/service-library/src/servicelib/redis/_errors.py b/packages/service-library/src/servicelib/redis/_errors.py index 998a9c1cb51..7fc3c7823ae 100644 --- a/packages/service-library/src/servicelib/redis/_errors.py +++ b/packages/service-library/src/servicelib/redis/_errors.py @@ -1,3 +1,6 @@ +from typing import TypeAlias + +import redis.exceptions from common_library.errors_classes import OsparcErrorMixin @@ -19,3 +22,6 @@ class LockLostError(BaseRedisError): "TIP: check connection to Redis DBs or look for Synchronous " "code that might block the auto-extender task. Somehow the distributed lock disappeared!" ) + + +ProjectLockError: TypeAlias = redis.exceptions.LockError # NOTE: backwards compatible diff --git a/packages/service-library/src/servicelib/redis/_project_lock.py b/packages/service-library/src/servicelib/redis/_project_lock.py index 651e24c784a..f634f87bbb6 100644 --- a/packages/service-library/src/servicelib/redis/_project_lock.py +++ b/packages/service-library/src/servicelib/redis/_project_lock.py @@ -1,19 +1,16 @@ import functools from collections.abc import Callable, Coroutine -from typing import Any, Final, ParamSpec, TypeAlias, TypeVar +from typing import Any, Final, ParamSpec, TypeVar -import redis -import redis.exceptions from models_library.projects import ProjectID from models_library.projects_access import Owner from models_library.projects_state import ProjectLocked, ProjectStatus from . import CouldNotAcquireLockError, RedisClientSDK, exclusive +from ._errors import ProjectLockError PROJECT_REDIS_LOCK_KEY: Final[str] = "project_lock:{}" -ProjectLockError: TypeAlias = redis.exceptions.LockError - P = ParamSpec("P") R = TypeVar("R") From cdabc03f520768cb999696e971562bf3992b28b5 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Jan 2025 22:14:34 +0100 Subject: [PATCH 13/28] refactor --- .../src/simcore_service_webserver/exporter/_handlers.py | 4 ++-- .../projects/_crud_api_create.py | 2 +- .../simcore_service_webserver/projects/projects_api.py | 8 ++++---- .../with_dbs/02/test_projects_crud_handlers__delete.py | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py index 63ff343fd06..a730150dbe7 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py @@ -12,7 +12,7 @@ from .._constants import RQ_PRODUCT_KEY from .._meta import API_VTAG from ..login.decorators import login_required -from ..projects.projects_api import with_project_locked_notified_state +from ..projects.projects_api import with_project_locked_and_notify from ..security.decorators import permission_required from ..users.api import get_user_fullname from ._formatter.archive import get_sds_archive_path @@ -43,7 +43,7 @@ async def export_project(request: web.Request): project_uuid = request.match_info.get("project_id") assert project_uuid # nosec - @with_project_locked_notified_state( + @with_project_locked_and_notify( request.app, project_uuid=project_uuid, status=ProjectStatus.EXPORTING, 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 e72c921ea66..9e56b7b7dcb 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 @@ -180,7 +180,7 @@ async def _copy() -> None: await long_running_task.result() if needs_lock_source_project: - await projects_api.with_project_locked_notified_state( + await projects_api.with_project_locked_and_notify( app, project_uuid=source_project["uuid"], status=ProjectStatus.CLONING, diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index a239a4fde78..f400be1aa83 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -1261,13 +1261,13 @@ async def try_open_project_for_user( """ try: - @with_project_locked_notified_state( + @with_project_locked_and_notify( app, project_uuid=project_uuid, status=ProjectStatus.OPENING, user_id=user_id, user_name=await get_user_fullname(app, user_id=user_id), - notify_users=False, + notify_users=True, ) async def _open_project() -> bool: with managed_resource(user_id, client_session_id, app) as user_session: @@ -1757,7 +1757,7 @@ async def remove_project_dynamic_services( save_state = False # ------------------- - @with_project_locked_notified_state( + @with_project_locked_and_notify( app, project_uuid=project_uuid, status=ProjectStatus.CLOSING, @@ -1864,7 +1864,7 @@ async def retrieve_and_notify_project_locked_state( R = TypeVar("R") -def with_project_locked_notified_state( +def with_project_locked_and_notify( app: web.Application, *, project_uuid: str, diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py index 1d489a606ea..24bd7d506bd 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py @@ -37,7 +37,7 @@ from simcore_service_webserver.projects import _crud_api_delete from simcore_service_webserver.projects.models import ProjectDict from simcore_service_webserver.projects.projects_api import ( - with_project_locked_notified_state, + with_project_locked_and_notify, ) from socketio.exceptions import ConnectionError as SocketConnectionError @@ -230,7 +230,7 @@ async def test_delete_project_while_it_is_locked_raises_error( project_uuid = user_project["uuid"] user_id = logged_user["id"] - await with_project_locked_notified_state( + await with_project_locked_and_notify( app=client.app, project_uuid=project_uuid, status=ProjectStatus.CLOSING, From 66b4f6755977ef1454a0bd3954d2e2e38bb5fc08 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 08:33:40 +0100 Subject: [PATCH 14/28] moving decorator out of api --- .../projects/_projects_locks_utils.py | 57 +++++++++++++++++++ .../projects/projects_api.py | 57 +------------------ 2 files changed, 60 insertions(+), 54 deletions(-) create mode 100644 services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py b/services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py new file mode 100644 index 00000000000..c666c274f1c --- /dev/null +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py @@ -0,0 +1,57 @@ +from collections.abc import Callable, Coroutine +from functools import wraps +from typing import Any, ParamSpec, TypeVar + +from aiohttp import web +from models_library.projects_access import Owner +from models_library.projects_state import ProjectStatus +from servicelib.redis._project_lock import with_project_locked + +from ..redis import get_redis_lock_manager_client_sdk +from ..users.api import FullNameDict +from .projects_api import retrieve_and_notify_project_locked_state + +P = ParamSpec("P") +R = TypeVar("R") + + +def with_project_locked_and_notify( + app: web.Application, + *, + project_uuid: str, + status: ProjectStatus, + user_id: int, + user_name: FullNameDict, + notify_users: bool, +) -> Callable[ + [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] +]: + def _decorator( + func: Callable[P, Coroutine[Any, Any, R]], + ) -> Callable[P, Coroutine[Any, Any, R]]: + @wraps(func) + async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: + @with_project_locked( + get_redis_lock_manager_client_sdk(app), + project_uuid=project_uuid, + status=status, + owner=Owner(user_id=user_id, **user_name), + ) + async def _locked_func() -> R: + if notify_users: + await retrieve_and_notify_project_locked_state( + user_id, project_uuid, app + ) + + return await func(*args, **kwargs) + + result = await _locked_func() + if notify_users: + await retrieve_and_notify_project_locked_state( + user_id, project_uuid, app + ) + return result + + return _wrapper + + return _decorator diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index f400be1aa83..5d3b66adbd0 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -13,12 +13,11 @@ import json import logging from collections import defaultdict -from collections.abc import Callable, Coroutine, Generator +from collections.abc import Generator from contextlib import suppress from decimal import Decimal -from functools import wraps from pprint import pformat -from typing import Any, Final, ParamSpec, TypeVar, cast +from typing import Any, Final, cast from uuid import UUID, uuid4 from aiohttp import web @@ -81,11 +80,7 @@ ServiceWaitingForManualInterventionError, ServiceWasNotFoundError, ) -from servicelib.redis import ( - get_project_locked_state, - is_project_locked, - with_project_locked, -) +from servicelib.redis import get_project_locked_state, is_project_locked from servicelib.redis._decorators import exclusive from servicelib.utils import fire_and_forget_task, logged_gather from simcore_postgres_database.models.users import UserRole @@ -1860,52 +1855,6 @@ async def retrieve_and_notify_project_locked_state( ) -P = ParamSpec("P") -R = TypeVar("R") - - -def with_project_locked_and_notify( - app: web.Application, - *, - project_uuid: str, - status: ProjectStatus, - user_id: int, - user_name: FullNameDict, - notify_users: bool, -) -> Callable[ - [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] -]: - def _decorator( - func: Callable[P, Coroutine[Any, Any, R]], - ) -> Callable[P, Coroutine[Any, Any, R]]: - @wraps(func) - async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: - @with_project_locked( - get_redis_lock_manager_client_sdk(app), - project_uuid=project_uuid, - status=status, - owner=Owner(user_id=user_id, **user_name), - ) - async def _locked_func() -> R: - if notify_users: - await retrieve_and_notify_project_locked_state( - user_id, project_uuid, app - ) - - return await func(*args, **kwargs) - - result = await _locked_func() - if notify_users: - await retrieve_and_notify_project_locked_state( - user_id, project_uuid, app - ) - return result - - return _wrapper - - return _decorator - - async def get_project_inactivity( app: web.Application, project_id: ProjectID ) -> GetProjectInactivityResponse: From e2ccbaf17b7581c6bba77256f502d05576e85a5e Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 08:52:29 +0100 Subject: [PATCH 15/28] moving decorator out of api --- .../exporter/_handlers.py | 13 +++++++---- .../projects/_crud_api_create.py | 13 +++++++---- .../projects/_projects_locks_utils.py | 23 +++++++------------ .../simcore_service_webserver/projects/api.py | 2 ++ .../projects/projects_api.py | 19 ++++++++++----- .../02/test_projects_crud_handlers__delete.py | 11 ++++----- 6 files changed, 46 insertions(+), 35 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py index a730150dbe7..c706b16405f 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py @@ -6,13 +6,15 @@ from aiofiles.tempfile import TemporaryDirectory as AioTemporaryDirectory from aiohttp import web +from models_library.projects_access import Owner from models_library.projects_state import ProjectStatus from servicelib.request_keys import RQT_USERID_KEY from .._constants import RQ_PRODUCT_KEY from .._meta import API_VTAG from ..login.decorators import login_required -from ..projects.projects_api import with_project_locked_and_notify +from ..projects.api import with_project_locked_and_notify +from ..projects.projects_api import retrieve_and_notify_project_locked_state from ..security.decorators import permission_required from ..users.api import get_user_fullname from ._formatter.archive import get_sds_archive_path @@ -47,9 +49,12 @@ async def export_project(request: web.Request): request.app, project_uuid=project_uuid, status=ProjectStatus.EXPORTING, - user_id=user_id, - user_name=await get_user_fullname(request.app, user_id=user_id), - notify_users=True, + owner=Owner( + user_id=user_id, **await get_user_fullname(request.app, user_id=user_id) + ), + notification_cb=retrieve_and_notify_project_locked_state( + user_id, project_uuid, request.app + ), ) async def _() -> tuple[Callable[[], Coroutine[Any, Any, None]], Path]: # @GitHK what is this supposed to be doing?? 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 9e56b7b7dcb..5380a59ff5e 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 @@ -9,6 +9,7 @@ from models_library.api_schemas_long_running_tasks.base import ProgressPercent from models_library.api_schemas_webserver.projects import ProjectGet from models_library.projects import ProjectID +from models_library.projects_access import Owner from models_library.projects_nodes_io import NodeID, NodeIDStr from models_library.projects_state import ProjectStatus from models_library.users import UserID @@ -39,6 +40,7 @@ from . import projects_api from ._metadata_api import set_project_ancestors from ._permalink_api import update_or_pop_permalink_in_project +from ._projects_locks_utils import with_project_locked_and_notify from .db import ProjectDBAPI from .exceptions import ( ParentNodeNotFoundError, @@ -180,13 +182,16 @@ async def _copy() -> None: await long_running_task.result() if needs_lock_source_project: - await projects_api.with_project_locked_and_notify( + await with_project_locked_and_notify( app, project_uuid=source_project["uuid"], status=ProjectStatus.CLONING, - user_id=user_id, - user_name=await get_user_fullname(app, user_id=user_id), - notify_users=True, + owner=Owner( + user_id=user_id, **await get_user_fullname(app, user_id=user_id) + ), + notification_cb=projects_api.retrieve_and_notify_project_locked_state( + user_id, source_project["uuid"], app + ), )(_copy)() else: await _copy() diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py b/services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py index c666c274f1c..b0a01765dc0 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py @@ -1,6 +1,6 @@ from collections.abc import Callable, Coroutine from functools import wraps -from typing import Any, ParamSpec, TypeVar +from typing import Any, Awaitable, ParamSpec, TypeVar from aiohttp import web from models_library.projects_access import Owner @@ -8,8 +8,6 @@ from servicelib.redis._project_lock import with_project_locked from ..redis import get_redis_lock_manager_client_sdk -from ..users.api import FullNameDict -from .projects_api import retrieve_and_notify_project_locked_state P = ParamSpec("P") R = TypeVar("R") @@ -20,9 +18,8 @@ def with_project_locked_and_notify( *, project_uuid: str, status: ProjectStatus, - user_id: int, - user_name: FullNameDict, - notify_users: bool, + owner: Owner, + notification_cb: Awaitable | None, ) -> Callable[ [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] ]: @@ -35,21 +32,17 @@ async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: get_redis_lock_manager_client_sdk(app), project_uuid=project_uuid, status=status, - owner=Owner(user_id=user_id, **user_name), + owner=owner, ) async def _locked_func() -> R: - if notify_users: - await retrieve_and_notify_project_locked_state( - user_id, project_uuid, app - ) + if notification_cb is not None: + await notification_cb return await func(*args, **kwargs) result = await _locked_func() - if notify_users: - await retrieve_and_notify_project_locked_state( - user_id, project_uuid, app - ) + if notification_cb is not None: + await notification_cb return result return _wrapper diff --git a/services/web/server/src/simcore_service_webserver/projects/api.py b/services/web/server/src/simcore_service_webserver/projects/api.py index ba5f5ae14fb..0252a01f0bf 100644 --- a/services/web/server/src/simcore_service_webserver/projects/api.py +++ b/services/web/server/src/simcore_service_webserver/projects/api.py @@ -11,6 +11,7 @@ ) from ._permalink_api import ProjectPermalink from ._permalink_api import register_factory as register_permalink_factory +from ._projects_locks_utils import with_project_locked_and_notify from ._wallets_api import ( check_project_financial_status, connect_wallet_to_project, @@ -27,6 +28,7 @@ "ProjectPermalink", "register_permalink_factory", "check_project_financial_status", + "with_project_locked_and_notify", ) diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index 5d3b66adbd0..84570c548ce 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -128,6 +128,7 @@ ) from ._db_utils import PermissionStr from ._nodes_utils import set_reservation_same_as_limit, validate_new_service_resources +from ._projects_locks_utils import with_project_locked_and_notify from ._wallets_api import connect_wallet_to_project, get_project_wallet from .db import APP_PROJECT_DBAPI, ProjectDBAPI from .exceptions import ( @@ -1260,9 +1261,12 @@ async def try_open_project_for_user( app, project_uuid=project_uuid, status=ProjectStatus.OPENING, - user_id=user_id, - user_name=await get_user_fullname(app, user_id=user_id), - notify_users=True, + owner=Owner( + user_id=user_id, **await get_user_fullname(app, user_id=user_id) + ), + notification_cb=retrieve_and_notify_project_locked_state( + user_id, project_uuid, app + ), ) async def _open_project() -> bool: with managed_resource(user_id, client_session_id, app) as user_session: @@ -1756,9 +1760,12 @@ async def remove_project_dynamic_services( app, project_uuid=project_uuid, status=ProjectStatus.CLOSING, - user_id=user_id, - user_name=user_name_data, - notify_users=notify_users, + owner=Owner(user_id=user_id, **user_name_data), + notification_cb=( + retrieve_and_notify_project_locked_state(user_id, project_uuid, app) + if notify_users + else None + ), ) async def _locked_stop_dynamic_serivces_in_project() -> None: # save the state if the user is not a guest. if we do not know we save in any case. diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py index 24bd7d506bd..28f34a02e8f 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py @@ -20,6 +20,7 @@ DynamicServiceStop, ) from models_library.projects import ProjectID +from models_library.projects_access import Owner from models_library.projects_state import ProjectStatus from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.webserver_login import UserInfoDict @@ -34,11 +35,9 @@ from simcore_postgres_database.models.projects_to_products import projects_to_products from simcore_service_webserver._meta import api_version_prefix from simcore_service_webserver.db.models import UserRole +from simcore_service_webserver.exporter._handlers import with_project_locked_and_notify from simcore_service_webserver.projects import _crud_api_delete from simcore_service_webserver.projects.models import ProjectDict -from simcore_service_webserver.projects.projects_api import ( - with_project_locked_and_notify, -) from socketio.exceptions import ConnectionError as SocketConnectionError @@ -225,6 +224,7 @@ async def test_delete_project_while_it_is_locked_raises_error( logged_user: UserInfoDict, user_project: ProjectDict, expected: ExpectedResponse, + faker: Faker, ): assert client.app @@ -234,7 +234,6 @@ async def test_delete_project_while_it_is_locked_raises_error( app=client.app, project_uuid=project_uuid, status=ProjectStatus.CLOSING, - user_id=user_id, - user_name={"first_name": "test", "last_name": "test"}, - notify_users=False, + owner=Owner(user_id=user_id, first_name=faker.name(), last_name=faker.name()), + notification_cb=None, )(_request_delete_project)(client, user_project, expected.conflict) From f77054ba299b2e0e86b17030c02c1524f0e05db5 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:55:54 +0100 Subject: [PATCH 16/28] fix test --- .../efs-guardian/tests/unit/test_efs_removal_policy_task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/efs-guardian/tests/unit/test_efs_removal_policy_task.py b/services/efs-guardian/tests/unit/test_efs_removal_policy_task.py index 30da8fd8cf8..4000fab0c88 100644 --- a/services/efs-guardian/tests/unit/test_efs_removal_policy_task.py +++ b/services/efs-guardian/tests/unit/test_efs_removal_policy_task.py @@ -120,7 +120,7 @@ def wrapper(*args, **kwargs): @patch("simcore_service_efs_guardian.services.background_tasks.get_redis_lock_client") @patch( - "simcore_service_efs_guardian.services.background_tasks.with_locked_project", + "simcore_service_efs_guardian.services.background_tasks.with_project_locked", new=mock_decorator, ) async def test_efs_removal_policy_task( From f36746caed6cec205e228355b195ebd356e18a20 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:56:07 +0100 Subject: [PATCH 17/28] fix cyclic import --- .../src/servicelib/redis/__init__.py | 2 -- .../src/servicelib/redis/_project_lock.py | 27 ++++++++++++++----- .../tests/redis/test_project_lock.py | 4 +-- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/service-library/src/servicelib/redis/__init__.py b/packages/service-library/src/servicelib/redis/__init__.py index 5f7fb2b4f31..9e63a9f6525 100644 --- a/packages/service-library/src/servicelib/redis/__init__.py +++ b/packages/service-library/src/servicelib/redis/__init__.py @@ -9,7 +9,6 @@ ) from ._models import RedisManagerDBConfig from ._project_lock import ( - PROJECT_REDIS_LOCK_KEY, get_project_locked_state, is_project_locked, with_project_locked, @@ -24,7 +23,6 @@ "handle_redis_returns_union_types", "is_project_locked", "LockLostError", - "PROJECT_REDIS_LOCK_KEY", "ProjectLockError", "RedisClientSDK", "RedisClientsManager", diff --git a/packages/service-library/src/servicelib/redis/_project_lock.py b/packages/service-library/src/servicelib/redis/_project_lock.py index f634f87bbb6..a312bfb5b50 100644 --- a/packages/service-library/src/servicelib/redis/_project_lock.py +++ b/packages/service-library/src/servicelib/redis/_project_lock.py @@ -6,10 +6,11 @@ from models_library.projects_access import Owner from models_library.projects_state import ProjectLocked, ProjectStatus -from . import CouldNotAcquireLockError, RedisClientSDK, exclusive -from ._errors import ProjectLockError +from ._client import RedisClientSDK +from ._decorators import exclusive +from ._errors import CouldNotAcquireLockError, ProjectLockError -PROJECT_REDIS_LOCK_KEY: Final[str] = "project_lock:{}" +_PROJECT_REDIS_LOCK_KEY: Final[str] = "project_lock:{}" P = ParamSpec("P") @@ -25,6 +26,20 @@ def with_project_locked( ) -> Callable[ [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] ]: + """creates a distributed auto sustained Redis lock for project with project_uuid, keeping its status and owner in the lock data + + Arguments: + redis_client -- the client to use to access redis + project_uuid -- the project UUID + status -- the project status + + Keyword Arguments: + owner -- the owner of the lock (default: {None}) + + Returns: + the decorated function return value + """ + def _decorator( func: Callable[P, Coroutine[Any, Any, R]], ) -> Callable[P, Coroutine[Any, Any, R]]: @@ -32,7 +47,7 @@ def _decorator( async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: @exclusive( redis_client, - lock_key=PROJECT_REDIS_LOCK_KEY.format(project_uuid), + lock_key=_PROJECT_REDIS_LOCK_KEY.format(project_uuid), lock_value=ProjectLocked( value=True, owner=owner, @@ -55,7 +70,7 @@ async def _exclusive_func(*args, **kwargs) -> R: async def is_project_locked( redis_client: RedisClientSDK, project_uuid: str | ProjectID ) -> bool: - redis_lock = redis_client.create_lock(PROJECT_REDIS_LOCK_KEY.format(project_uuid)) + redis_lock = redis_client.create_lock(_PROJECT_REDIS_LOCK_KEY.format(project_uuid)) return await redis_lock.locked() @@ -68,7 +83,7 @@ async def get_project_locked_state( """ if await is_project_locked(redis_client, project_uuid=project_uuid) and ( lock_value := await redis_client.redis.get( - PROJECT_REDIS_LOCK_KEY.format(project_uuid) + _PROJECT_REDIS_LOCK_KEY.format(project_uuid) ) ): return ProjectLocked.model_validate_json(lock_value) diff --git a/packages/service-library/tests/redis/test_project_lock.py b/packages/service-library/tests/redis/test_project_lock.py index e22e8ae9b3e..643aae22468 100644 --- a/packages/service-library/tests/redis/test_project_lock.py +++ b/packages/service-library/tests/redis/test_project_lock.py @@ -15,13 +15,13 @@ from models_library.projects_state import ProjectLocked, ProjectStatus from servicelib.async_utils import cancel_wait_task from servicelib.redis import ( - PROJECT_REDIS_LOCK_KEY, ProjectLockError, RedisClientSDK, get_project_locked_state, is_project_locked, with_project_locked, ) +from servicelib.redis._project_lock import _PROJECT_REDIS_LOCK_KEY pytest_simcore_core_services_selection = [ "redis", @@ -79,7 +79,7 @@ async def _locked_fct() -> None: ) # check lock name formatting is correct redis_lock = await redis_client_sdk.redis.get( - PROJECT_REDIS_LOCK_KEY.format(project_uuid) + _PROJECT_REDIS_LOCK_KEY.format(project_uuid) ) assert redis_lock assert ProjectLocked.model_validate_json(redis_lock) == ProjectLocked( From ee5d0332c273f8ea63688fd84398d0aa9ab37571 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:58:08 +0100 Subject: [PATCH 18/28] fixed pylint --- .../src/simcore_service_webserver/exporter/_handlers.py | 1 - .../src/simcore_service_webserver/projects/_crud_handlers.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py index c706b16405f..d6b04f30b27 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py @@ -78,7 +78,6 @@ async def _() -> tuple[Callable[[], Coroutine[Any, Any, None]], Path]: delete_tmp_dir_callable, file_to_download = await _() headers = {"Content-Disposition": f'attachment; filename="{file_to_download.name}"'} - assert delete_tmp_dir_callable # nosec return CleanupFileResponse( remove_tmp_dir_cb=delete_tmp_dir_callable, path=file_to_download, diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index 53e186d399b..2530db3053d 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -140,8 +140,8 @@ async def create_project(request: web.Request): project_create: ( ProjectCreateNew | ProjectCopyOverride | EmptyModel ) = await parse_request_body_as( - ProjectCreateNew | ProjectCopyOverride | EmptyModel, - request, # type: ignore[arg-type] # from pydantic v2 --> https://github.com/pydantic/pydantic/discussions/4950 + ProjectCreateNew | ProjectCopyOverride | EmptyModel, # type: ignore[arg-type] # from pydantic v2 --> https://github.com/pydantic/pydantic/discussions/4950 + request, ) predefined_project = ( project_create.model_dump( From 77ac6cd2351338d8254c7fdef797422fbd6b1eab Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:07:12 +0100 Subject: [PATCH 19/28] separating dependencies --- .../src/servicelib/redis/_project_lock.py | 39 ++++++++++++++- .../exporter/_handlers.py | 3 +- .../projects/_crud_api_create.py | 15 ++++-- .../projects/_projects_locks_utils.py | 50 ------------------- .../simcore_service_webserver/projects/api.py | 2 - .../projects/projects_api.py | 6 +-- 6 files changed, 53 insertions(+), 62 deletions(-) delete mode 100644 services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py diff --git a/packages/service-library/src/servicelib/redis/_project_lock.py b/packages/service-library/src/servicelib/redis/_project_lock.py index a312bfb5b50..8e9e7ece687 100644 --- a/packages/service-library/src/servicelib/redis/_project_lock.py +++ b/packages/service-library/src/servicelib/redis/_project_lock.py @@ -1,5 +1,5 @@ import functools -from collections.abc import Callable, Coroutine +from collections.abc import Awaitable, Callable, Coroutine from typing import Any, Final, ParamSpec, TypeVar from models_library.projects import ProjectID @@ -67,6 +67,43 @@ async def _exclusive_func(*args, **kwargs) -> R: return _decorator +def with_project_locked_and_notify( + redis_client: RedisClientSDK | Callable[..., RedisClientSDK], + *, + project_uuid: str, + status: ProjectStatus, + owner: Owner, + notification_cb: Callable[[], Awaitable[None]] | None, +) -> Callable[ + [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] +]: + def _decorator( + func: Callable[P, Coroutine[Any, Any, R]], + ) -> Callable[P, Coroutine[Any, Any, R]]: + @functools.wraps(func) + async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: + @with_project_locked( + redis_client, + project_uuid=project_uuid, + status=status, + owner=owner, + ) + async def _locked_func() -> R: + if notification_cb is not None: + await notification_cb() + + return await func(*args, **kwargs) + + result = await _locked_func() + if notification_cb is not None: + await notification_cb() + return result + + return _wrapper + + return _decorator + + async def is_project_locked( redis_client: RedisClientSDK, project_uuid: str | ProjectID ) -> bool: diff --git a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py index d6b04f30b27..db081957d33 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py @@ -15,6 +15,7 @@ from ..login.decorators import login_required from ..projects.api import with_project_locked_and_notify from ..projects.projects_api import retrieve_and_notify_project_locked_state +from ..redis import get_redis_lock_manager_client_sdk from ..security.decorators import permission_required from ..users.api import get_user_fullname from ._formatter.archive import get_sds_archive_path @@ -46,7 +47,7 @@ async def export_project(request: web.Request): assert project_uuid # nosec @with_project_locked_and_notify( - request.app, + get_redis_lock_manager_client_sdk(request.app), project_uuid=project_uuid, status=ProjectStatus.EXPORTING, owner=Owner( 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 5380a59ff5e..b173a310ea3 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 pydantic import TypeAdapter from servicelib.aiohttp.long_running_tasks.server import TaskProgress from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON +from servicelib.redis._project_lock import with_project_locked_and_notify from simcore_postgres_database.utils_projects_nodes import ( ProjectNode, ProjectNodeCreate, @@ -29,6 +30,7 @@ from ..director_v2 import api as director_v2_api from ..dynamic_scheduler import api as dynamic_scheduler_api from ..folders import _folders_repository as folders_db +from ..redis import get_redis_lock_manager_client_sdk from ..storage.api import ( copy_data_folders_from_project, get_project_total_size_simcore_s3, @@ -40,7 +42,6 @@ from . import projects_api from ._metadata_api import set_project_ancestors from ._permalink_api import update_or_pop_permalink_in_project -from ._projects_locks_utils import with_project_locked_and_notify from .db import ProjectDBAPI from .exceptions import ( ParentNodeNotFoundError, @@ -182,16 +183,20 @@ async def _copy() -> None: await long_running_task.result() if needs_lock_source_project: + + async def _notification_cb() -> None: + await projects_api.retrieve_and_notify_project_locked_state( + user_id, source_project["uuid"], app + ) + await with_project_locked_and_notify( - app, + get_redis_lock_manager_client_sdk(app), project_uuid=source_project["uuid"], status=ProjectStatus.CLONING, owner=Owner( user_id=user_id, **await get_user_fullname(app, user_id=user_id) ), - notification_cb=projects_api.retrieve_and_notify_project_locked_state( - user_id, source_project["uuid"], app - ), + notification_cb=_notification_cb, )(_copy)() else: await _copy() diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py b/services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py deleted file mode 100644 index b0a01765dc0..00000000000 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_locks_utils.py +++ /dev/null @@ -1,50 +0,0 @@ -from collections.abc import Callable, Coroutine -from functools import wraps -from typing import Any, Awaitable, ParamSpec, TypeVar - -from aiohttp import web -from models_library.projects_access import Owner -from models_library.projects_state import ProjectStatus -from servicelib.redis._project_lock import with_project_locked - -from ..redis import get_redis_lock_manager_client_sdk - -P = ParamSpec("P") -R = TypeVar("R") - - -def with_project_locked_and_notify( - app: web.Application, - *, - project_uuid: str, - status: ProjectStatus, - owner: Owner, - notification_cb: Awaitable | None, -) -> Callable[ - [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] -]: - def _decorator( - func: Callable[P, Coroutine[Any, Any, R]], - ) -> Callable[P, Coroutine[Any, Any, R]]: - @wraps(func) - async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: - @with_project_locked( - get_redis_lock_manager_client_sdk(app), - project_uuid=project_uuid, - status=status, - owner=owner, - ) - async def _locked_func() -> R: - if notification_cb is not None: - await notification_cb - - return await func(*args, **kwargs) - - result = await _locked_func() - if notification_cb is not None: - await notification_cb - return result - - return _wrapper - - return _decorator diff --git a/services/web/server/src/simcore_service_webserver/projects/api.py b/services/web/server/src/simcore_service_webserver/projects/api.py index 0252a01f0bf..ba5f5ae14fb 100644 --- a/services/web/server/src/simcore_service_webserver/projects/api.py +++ b/services/web/server/src/simcore_service_webserver/projects/api.py @@ -11,7 +11,6 @@ ) from ._permalink_api import ProjectPermalink from ._permalink_api import register_factory as register_permalink_factory -from ._projects_locks_utils import with_project_locked_and_notify from ._wallets_api import ( check_project_financial_status, connect_wallet_to_project, @@ -28,7 +27,6 @@ "ProjectPermalink", "register_permalink_factory", "check_project_financial_status", - "with_project_locked_and_notify", ) diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index 84570c548ce..a816bbf0710 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -82,6 +82,7 @@ ) from servicelib.redis import get_project_locked_state, is_project_locked from servicelib.redis._decorators import exclusive +from servicelib.redis._project_lock import with_project_locked_and_notify from servicelib.utils import fire_and_forget_task, logged_gather from simcore_postgres_database.models.users import UserRole from simcore_postgres_database.utils_projects_nodes import ( @@ -128,7 +129,6 @@ ) from ._db_utils import PermissionStr from ._nodes_utils import set_reservation_same_as_limit, validate_new_service_resources -from ._projects_locks_utils import with_project_locked_and_notify from ._wallets_api import connect_wallet_to_project, get_project_wallet from .db import APP_PROJECT_DBAPI, ProjectDBAPI from .exceptions import ( @@ -1258,7 +1258,7 @@ async def try_open_project_for_user( try: @with_project_locked_and_notify( - app, + get_redis_lock_manager_client_sdk(app), project_uuid=project_uuid, status=ProjectStatus.OPENING, owner=Owner( @@ -1757,7 +1757,7 @@ async def remove_project_dynamic_services( # ------------------- @with_project_locked_and_notify( - app, + get_redis_lock_manager_client_sdk(app), project_uuid=project_uuid, status=ProjectStatus.CLOSING, owner=Owner(user_id=user_id, **user_name_data), From a0f14433ddb33d98c18eea977c33de8fa590e091 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:13:05 +0100 Subject: [PATCH 20/28] cleanup --- .../src/servicelib/redis/_project_lock.py | 51 ++++--------------- .../rpc/clusters.py | 3 +- .../services/process_messages.py | 2 +- .../exporter/_handlers.py | 4 +- .../projects/_crud_api_create.py | 4 +- .../projects/projects_api.py | 13 +++-- .../02/test_projects_crud_handlers__delete.py | 4 +- 7 files changed, 26 insertions(+), 55 deletions(-) diff --git a/packages/service-library/src/servicelib/redis/_project_lock.py b/packages/service-library/src/servicelib/redis/_project_lock.py index 8e9e7ece687..c762251f2f9 100644 --- a/packages/service-library/src/servicelib/redis/_project_lock.py +++ b/packages/service-library/src/servicelib/redis/_project_lock.py @@ -22,7 +22,8 @@ def with_project_locked( *, project_uuid: str | ProjectID, status: ProjectStatus, - owner: Owner | None = None, + owner: Owner | None, + notification_cb: Callable[[], Awaitable[None]] | None, ) -> Callable[ [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] ]: @@ -32,9 +33,8 @@ def with_project_locked( redis_client -- the client to use to access redis project_uuid -- the project UUID status -- the project status - - Keyword Arguments: owner -- the owner of the lock (default: {None}) + notification_cb -- a notification callback that will be called AFTER the project is locked and AFTER it was unlocked Returns: the decorated function return value @@ -55,49 +55,18 @@ async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: ).model_dump_json(), ) async def _exclusive_func(*args, **kwargs) -> R: + if notification_cb is not None: + await notification_cb() return await func(*args, **kwargs) try: - return await _exclusive_func(*args, **kwargs) - except CouldNotAcquireLockError as e: - raise ProjectLockError from e - - return _wrapper - - return _decorator - - -def with_project_locked_and_notify( - redis_client: RedisClientSDK | Callable[..., RedisClientSDK], - *, - project_uuid: str, - status: ProjectStatus, - owner: Owner, - notification_cb: Callable[[], Awaitable[None]] | None, -) -> Callable[ - [Callable[P, Coroutine[Any, Any, R]]], Callable[P, Coroutine[Any, Any, R]] -]: - def _decorator( - func: Callable[P, Coroutine[Any, Any, R]], - ) -> Callable[P, Coroutine[Any, Any, R]]: - @functools.wraps(func) - async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R: - @with_project_locked( - redis_client, - project_uuid=project_uuid, - status=status, - owner=owner, - ) - async def _locked_func() -> R: + result = await _exclusive_func(*args, **kwargs) + # we are now unlocked if notification_cb is not None: await notification_cb() - - return await func(*args, **kwargs) - - result = await _locked_func() - if notification_cb is not None: - await notification_cb() - return result + return result + except CouldNotAcquireLockError as e: + raise ProjectLockError from e return _wrapper diff --git a/services/clusters-keeper/src/simcore_service_clusters_keeper/rpc/clusters.py b/services/clusters-keeper/src/simcore_service_clusters_keeper/rpc/clusters.py index b7552dcfb09..82f84b9d471 100644 --- a/services/clusters-keeper/src/simcore_service_clusters_keeper/rpc/clusters.py +++ b/services/clusters-keeper/src/simcore_service_clusters_keeper/rpc/clusters.py @@ -7,8 +7,7 @@ from models_library.users import UserID from models_library.wallets import WalletID from servicelib.rabbitmq import RPCRouter -from servicelib.redis._client import RedisClientSDK -from servicelib.redis._decorators import exclusive +from servicelib.redis import RedisClientSDK, exclusive from ..core.settings import get_application_settings from ..modules import clusters diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages.py index 77907bc51a5..1b84c02df1d 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages.py @@ -9,7 +9,7 @@ from servicelib.rabbitmq.rpc_interfaces.dynamic_sidecar.disk_usage import ( update_disk_usage, ) -from servicelib.redis._decorators import exclusive +from servicelib.redis import exclusive from servicelib.utils import fire_and_forget_task from ..core.settings import get_application_settings diff --git a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py index db081957d33..46050b213f9 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py @@ -8,12 +8,12 @@ from aiohttp import web from models_library.projects_access import Owner from models_library.projects_state import ProjectStatus +from servicelib.redis import with_project_locked from servicelib.request_keys import RQT_USERID_KEY from .._constants import RQ_PRODUCT_KEY from .._meta import API_VTAG from ..login.decorators import login_required -from ..projects.api import with_project_locked_and_notify from ..projects.projects_api import retrieve_and_notify_project_locked_state from ..redis import get_redis_lock_manager_client_sdk from ..security.decorators import permission_required @@ -46,7 +46,7 @@ async def export_project(request: web.Request): project_uuid = request.match_info.get("project_id") assert project_uuid # nosec - @with_project_locked_and_notify( + @with_project_locked( get_redis_lock_manager_client_sdk(request.app), project_uuid=project_uuid, status=ProjectStatus.EXPORTING, 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 b173a310ea3..8f3fe7c3903 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,7 +18,7 @@ from pydantic import TypeAdapter from servicelib.aiohttp.long_running_tasks.server import TaskProgress from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON -from servicelib.redis._project_lock import with_project_locked_and_notify +from servicelib.redis import with_project_locked from simcore_postgres_database.utils_projects_nodes import ( ProjectNode, ProjectNodeCreate, @@ -189,7 +189,7 @@ async def _notification_cb() -> None: user_id, source_project["uuid"], app ) - await with_project_locked_and_notify( + await with_project_locked( get_redis_lock_manager_client_sdk(app), project_uuid=source_project["uuid"], status=ProjectStatus.CLONING, diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index a816bbf0710..c86cca9b9d1 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -80,9 +80,12 @@ ServiceWaitingForManualInterventionError, ServiceWasNotFoundError, ) -from servicelib.redis import get_project_locked_state, is_project_locked -from servicelib.redis._decorators import exclusive -from servicelib.redis._project_lock import with_project_locked_and_notify +from servicelib.redis import ( + exclusive, + get_project_locked_state, + is_project_locked, + with_project_locked, +) from servicelib.utils import fire_and_forget_task, logged_gather from simcore_postgres_database.models.users import UserRole from simcore_postgres_database.utils_projects_nodes import ( @@ -1257,7 +1260,7 @@ async def try_open_project_for_user( """ try: - @with_project_locked_and_notify( + @with_project_locked( get_redis_lock_manager_client_sdk(app), project_uuid=project_uuid, status=ProjectStatus.OPENING, @@ -1756,7 +1759,7 @@ async def remove_project_dynamic_services( save_state = False # ------------------- - @with_project_locked_and_notify( + @with_project_locked( get_redis_lock_manager_client_sdk(app), project_uuid=project_uuid, status=ProjectStatus.CLOSING, diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py index 28f34a02e8f..4226a16914c 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py @@ -31,11 +31,11 @@ ) from servicelib.aiohttp import status from servicelib.common_headers import UNDEFINED_DEFAULT_SIMCORE_USER_AGENT_VALUE +from servicelib.redis import with_project_locked from simcore_postgres_database.models.products import products from simcore_postgres_database.models.projects_to_products import projects_to_products from simcore_service_webserver._meta import api_version_prefix from simcore_service_webserver.db.models import UserRole -from simcore_service_webserver.exporter._handlers import with_project_locked_and_notify from simcore_service_webserver.projects import _crud_api_delete from simcore_service_webserver.projects.models import ProjectDict from socketio.exceptions import ConnectionError as SocketConnectionError @@ -230,7 +230,7 @@ async def test_delete_project_while_it_is_locked_raises_error( project_uuid = user_project["uuid"] user_id = logged_user["id"] - await with_project_locked_and_notify( + await with_project_locked( app=client.app, project_uuid=project_uuid, status=ProjectStatus.CLOSING, From 62526e4d20da3991f184fbf2b4cd9cb1884d2c5a Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:13:57 +0100 Subject: [PATCH 21/28] doc --- .../service-library/src/servicelib/redis/_project_lock.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/service-library/src/servicelib/redis/_project_lock.py b/packages/service-library/src/servicelib/redis/_project_lock.py index c762251f2f9..12f25e068d9 100644 --- a/packages/service-library/src/servicelib/redis/_project_lock.py +++ b/packages/service-library/src/servicelib/redis/_project_lock.py @@ -34,10 +34,13 @@ def with_project_locked( project_uuid -- the project UUID status -- the project status owner -- the owner of the lock (default: {None}) - notification_cb -- a notification callback that will be called AFTER the project is locked and AFTER it was unlocked + notification_cb -- an optional notification callback that will be called AFTER the project is locked and AFTER it was unlocked Returns: the decorated function return value + + Raises: + raises anything from the decorated function and from the optional notification callback """ def _decorator( From b2e40a5f89cbc01b755558fcda9701f8df14a52c Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:23:16 +0100 Subject: [PATCH 22/28] refactor --- .../exporter/_handlers.py | 7 +++--- .../projects/_crud_api_create.py | 10 +++----- .../projects/_states_handlers.py | 6 ++--- .../projects/projects_api.py | 23 ++++++++++++------- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py index 46050b213f9..9e64374d6a8 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py @@ -6,6 +6,7 @@ from aiofiles.tempfile import TemporaryDirectory as AioTemporaryDirectory from aiohttp import web +from models_library.projects import ProjectID from models_library.projects_access import Owner from models_library.projects_state import ProjectStatus from servicelib.redis import with_project_locked @@ -14,7 +15,7 @@ from .._constants import RQ_PRODUCT_KEY from .._meta import API_VTAG from ..login.decorators import login_required -from ..projects.projects_api import retrieve_and_notify_project_locked_state +from ..projects.projects_api import create_user_notification_cb from ..redis import get_redis_lock_manager_client_sdk from ..security.decorators import permission_required from ..users.api import get_user_fullname @@ -53,8 +54,8 @@ async def export_project(request: web.Request): owner=Owner( user_id=user_id, **await get_user_fullname(request.app, user_id=user_id) ), - notification_cb=retrieve_and_notify_project_locked_state( - user_id, project_uuid, request.app + notification_cb=create_user_notification_cb( + user_id, ProjectID(f"{project_uuid}"), request.app ), ) async def _() -> tuple[Callable[[], Coroutine[Any, Any, None]], Path]: 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 8f3fe7c3903..67d11cfc010 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 @@ -183,12 +183,6 @@ async def _copy() -> None: await long_running_task.result() if needs_lock_source_project: - - async def _notification_cb() -> None: - await projects_api.retrieve_and_notify_project_locked_state( - user_id, source_project["uuid"], app - ) - await with_project_locked( get_redis_lock_manager_client_sdk(app), project_uuid=source_project["uuid"], @@ -196,7 +190,9 @@ async def _notification_cb() -> None: owner=Owner( user_id=user_id, **await get_user_fullname(app, user_id=user_id) ), - notification_cb=_notification_cb, + notification_cb=projects_api.create_user_notification_cb( + user_id, ProjectID(f"{source_project['uuid']}"), app + ), )(_copy)() else: await _copy() diff --git a/services/web/server/src/simcore_service_webserver/projects/_states_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_states_handlers.py index bab3558b3cc..d1849918804 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_states_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_states_handlers.py @@ -1,6 +1,4 @@ -""" handlers for project states - -""" +"""handlers for project states""" import contextlib import functools @@ -144,7 +142,7 @@ async def open_project(request: web.Request) -> web.Response: if not await projects_api.try_open_project_for_user( req_ctx.user_id, - project_uuid=f"{path_params.project_id}", + project_uuid=path_params.project_id, client_session_id=client_session_id, app=request.app, max_number_of_studies_per_user=product.max_open_studies_per_user, diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index c86cca9b9d1..3267f8a8fad 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -1244,9 +1244,18 @@ async def _clean_user_disconnected_clients( await user_session.remove(PROJECT_ID_KEY) +def create_user_notification_cb( + user_id: UserID, project_uuid: ProjectID, app: web.Application +): + async def _notification_cb() -> None: + await retrieve_and_notify_project_locked_state(user_id, f"{project_uuid}", app) + + return _notification_cb + + async def try_open_project_for_user( user_id: UserID, - project_uuid: str, + project_uuid: ProjectID, client_session_id: str, app: web.Application, max_number_of_studies_per_user: int | None, @@ -1267,9 +1276,7 @@ async def try_open_project_for_user( owner=Owner( user_id=user_id, **await get_user_fullname(app, user_id=user_id) ), - notification_cb=retrieve_and_notify_project_locked_state( - user_id, project_uuid, app - ), + notification_cb=create_user_notification_cb(user_id, project_uuid, app), ) async def _open_project() -> bool: with managed_resource(user_id, client_session_id, app) as user_session: @@ -1296,11 +1303,11 @@ async def _open_project() -> bool: sessions_with_project: list[ UserSessionID ] = await user_session.find_users_of_resource( - app, PROJECT_ID_KEY, project_uuid + app, PROJECT_ID_KEY, f"{project_uuid}" ) if not sessions_with_project: # no one has the project so we assign it - await user_session.add(PROJECT_ID_KEY, project_uuid) + await user_session.add(PROJECT_ID_KEY, f"{project_uuid}") return True # Otherwise if this is the only user (NOTE: a session = user_id + client_seesion_id !) @@ -1316,7 +1323,7 @@ async def _open_project() -> bool: app, ): # steal the project - await user_session.add(PROJECT_ID_KEY, project_uuid) + await user_session.add(PROJECT_ID_KEY, f"{project_uuid}") await _clean_user_disconnected_clients( sessions_with_project, app ) @@ -1765,7 +1772,7 @@ async def remove_project_dynamic_services( status=ProjectStatus.CLOSING, owner=Owner(user_id=user_id, **user_name_data), notification_cb=( - retrieve_and_notify_project_locked_state(user_id, project_uuid, app) + create_user_notification_cb(user_id, ProjectID(project_uuid), app) if notify_users else None ), From e144b5cb99bba09bb262fe9b5389c87ce2623603 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:30:27 +0100 Subject: [PATCH 23/28] fix usage --- packages/service-library/tests/redis/test_project_lock.py | 2 ++ .../services/background_tasks.py | 2 ++ .../unit/with_dbs/02/test_projects_crud_handlers__delete.py | 5 ++--- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/service-library/tests/redis/test_project_lock.py b/packages/service-library/tests/redis/test_project_lock.py index 643aae22468..c1e12c96828 100644 --- a/packages/service-library/tests/redis/test_project_lock.py +++ b/packages/service-library/tests/redis/test_project_lock.py @@ -67,6 +67,7 @@ async def test_with_project_locked( project_uuid=project_uuid, status=project_status, owner=owner, + notification_cb=None, ) async def _locked_fct() -> None: assert await is_project_locked(redis_client_sdk, project_uuid) is True @@ -118,6 +119,7 @@ async def test_lock_already_locked_project_raises( project_uuid=project_uuid, status=project_status, owner=owner, + notification_cb=None, ) async def _locked_fct() -> None: started_event.set() diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py index fca4a2fafde..75ed9f66fc3 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py @@ -25,6 +25,8 @@ async def _lock_project_and_remove_data(app: FastAPI, project_id: ProjectID) -> get_redis_lock_client(app), project_uuid=project_id, status=ProjectStatus.MAINTAINING, + owner=None, + notification_cb=None, ) async def _remove(): await efs_manager.remove_project_efs_data(project_id) diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py index 4226a16914c..6dbcbe488ac 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__delete.py @@ -11,7 +11,6 @@ from unittest.mock import MagicMock, call import pytest -import redis.asyncio as aioredis import sqlalchemy as sa from aiohttp.test_utils import TestClient from faker import Faker @@ -38,6 +37,7 @@ from simcore_service_webserver.db.models import UserRole from simcore_service_webserver.projects import _crud_api_delete from simcore_service_webserver.projects.models import ProjectDict +from simcore_service_webserver.redis import get_redis_lock_manager_client_sdk from socketio.exceptions import ConnectionError as SocketConnectionError @@ -148,7 +148,6 @@ async def test_delete_multiple_opened_project_forbidden( user_role: UserRole, expected_ok: HTTPStatus, expected_forbidden: HTTPStatus, - redis_client: aioredis.Redis, ): assert client.app @@ -231,7 +230,7 @@ async def test_delete_project_while_it_is_locked_raises_error( project_uuid = user_project["uuid"] user_id = logged_user["id"] await with_project_locked( - app=client.app, + get_redis_lock_manager_client_sdk(client.app), project_uuid=project_uuid, status=ProjectStatus.CLOSING, owner=Owner(user_id=user_id, first_name=faker.name(), last_name=faker.name()), From e33c1f2af65e11f0b07fbb90930c36af1d08f70f Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:34:27 +0100 Subject: [PATCH 24/28] added test for notification --- .../tests/redis/test_project_lock.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/service-library/tests/redis/test_project_lock.py b/packages/service-library/tests/redis/test_project_lock.py index c1e12c96828..ba2ca05bf55 100644 --- a/packages/service-library/tests/redis/test_project_lock.py +++ b/packages/service-library/tests/redis/test_project_lock.py @@ -46,6 +46,14 @@ def owner(request: pytest.FixtureRequest) -> Owner: return Owner(**request.param) +from unittest import mock + + +@pytest.fixture +def mocked_notification_cb() -> mock.AsyncMock: + return mock.AsyncMock() + + @pytest.mark.parametrize( "project_status", [ @@ -61,15 +69,17 @@ async def test_with_project_locked( project_uuid: ProjectID, owner: Owner, project_status: ProjectStatus, + mocked_notification_cb: mock.AsyncMock, ): @with_project_locked( redis_client_sdk, project_uuid=project_uuid, status=project_status, owner=owner, - notification_cb=None, + notification_cb=mocked_notification_cb, ) async def _locked_fct() -> None: + mocked_notification_cb.assert_called_once() assert await is_project_locked(redis_client_sdk, project_uuid) is True locked_state = await get_project_locked_state(redis_client_sdk, project_uuid) assert locked_state is not None @@ -89,11 +99,14 @@ async def _locked_fct() -> None: status=project_status, ) + mocked_notification_cb.assert_not_called() assert await get_project_locked_state(redis_client_sdk, project_uuid) is None assert await is_project_locked(redis_client_sdk, project_uuid) is False await _locked_fct() assert await is_project_locked(redis_client_sdk, project_uuid) is False assert await get_project_locked_state(redis_client_sdk, project_uuid) is None + mocked_notification_cb.assert_called() + assert mocked_notification_cb.call_count == 2 @pytest.mark.parametrize( From 3060469c66a5b79e2ac88f676b95907a4dbcb349 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:37:00 +0100 Subject: [PATCH 25/28] fix usage --- packages/service-library/tests/redis/test_project_lock.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/service-library/tests/redis/test_project_lock.py b/packages/service-library/tests/redis/test_project_lock.py index ba2ca05bf55..aa9d7fd1c74 100644 --- a/packages/service-library/tests/redis/test_project_lock.py +++ b/packages/service-library/tests/redis/test_project_lock.py @@ -6,6 +6,7 @@ import asyncio from typing import cast +from unittest import mock from uuid import UUID import pytest @@ -46,9 +47,6 @@ def owner(request: pytest.FixtureRequest) -> Owner: return Owner(**request.param) -from unittest import mock - - @pytest.fixture def mocked_notification_cb() -> mock.AsyncMock: return mock.AsyncMock() From 787c638013340ef9cdb8ed4df1c66565514d1c1e Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 15:42:19 +0100 Subject: [PATCH 26/28] done --- .../server/src/simcore_service_webserver/exporter/_handlers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py index 9e64374d6a8..07821c489f4 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py @@ -59,7 +59,6 @@ async def export_project(request: web.Request): ), ) async def _() -> tuple[Callable[[], Coroutine[Any, Any, None]], Path]: - # @GitHK what is this supposed to be doing?? async with AsyncExitStack() as tmp_dir_stack: tmp_dir = await tmp_dir_stack.enter_async_context(AioTemporaryDirectory()) file_to_download = await get_sds_archive_path( From 730dcb1ea91daef5146b3102f95e9538350d276b Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Jan 2025 16:10:29 +0100 Subject: [PATCH 27/28] revert change --- .../simcore_service_webserver/projects/projects_api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/projects/projects_api.py b/services/web/server/src/simcore_service_webserver/projects/projects_api.py index 3267f8a8fad..8d421b9ad42 100644 --- a/services/web/server/src/simcore_service_webserver/projects/projects_api.py +++ b/services/web/server/src/simcore_service_webserver/projects/projects_api.py @@ -381,9 +381,9 @@ async def _get_default_pricing_and_hardware_info( ) -_MACHINE_TOTAL_RAM_SAFE_MARGIN_RATIO: Final[ - float -] = 0.1 # NOTE: machines always have less available RAM than advertised +_MACHINE_TOTAL_RAM_SAFE_MARGIN_RATIO: Final[float] = ( + 0.1 # NOTE: machines always have less available RAM than advertised +) _SIDECARS_OPS_SAFE_RAM_MARGIN: Final[ByteSize] = TypeAdapter(ByteSize).validate_python( "1GiB" ) @@ -1276,7 +1276,7 @@ async def try_open_project_for_user( owner=Owner( user_id=user_id, **await get_user_fullname(app, user_id=user_id) ), - notification_cb=create_user_notification_cb(user_id, project_uuid, app), + notification_cb=None, ) async def _open_project() -> bool: with managed_resource(user_id, client_session_id, app) as user_session: From 7f0986d3caf7245e279b94158ee08d755a5e040c Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Tue, 21 Jan 2025 11:02:13 +0100 Subject: [PATCH 28/28] fix code --- .../server/src/simcore_service_webserver/exporter/_handlers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py index 07821c489f4..59321710751 100644 --- a/services/web/server/src/simcore_service_webserver/exporter/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/exporter/_handlers.py @@ -74,7 +74,8 @@ async def _() -> tuple[Callable[[], Coroutine[Any, Any, None]], Path]: msg = f"Must provide a file to download, not {file_to_download!s}" raise SDSException(msg) # this allows to transfer deletion of the tmp dir responsibility - return tmp_dir_stack.pop_all().aclose, file_to_download + delete_tmp_dir_cb = tmp_dir_stack.pop_all().aclose + return delete_tmp_dir_cb, file_to_download delete_tmp_dir_callable, file_to_download = await _()