From 4219904b6e557b2c41fc88e197e6815ece9037bb Mon Sep 17 00:00:00 2001 From: matusdrobuliak66 Date: Wed, 23 Oct 2024 10:12:01 +0200 Subject: [PATCH 1/7] fix & improvements: --- .../models-library/src/models_library/projects_state.py | 8 -------- .../services/background_tasks.py | 4 ++++ .../services/background_tasks_setup.py | 6 +++--- .../services/process_messages_setup.py | 2 +- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/models-library/src/models_library/projects_state.py b/packages/models-library/src/models_library/projects_state.py index 757704e14d33..07b774805a09 100644 --- a/packages/models-library/src/models_library/projects_state.py +++ b/packages/models-library/src/models_library/projects_state.py @@ -80,14 +80,6 @@ class Config: ] } - @validator("owner", pre=True, always=True) - @classmethod - def check_not_null(cls, v, values): - if values["value"] is True and v is None: - msg = "value cannot be None when project is locked" - raise ValueError(msg) - return v - @validator("status", always=True) @classmethod def check_status_compatible(cls, v, values): 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 e6a9ee6ee15c..8ac475a4742a 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 @@ -34,6 +34,10 @@ async def removal_policy_task(app: FastAPI) -> None: efs_project_ids: list[ ProjectID ] = await efs_manager.list_projects_across_whole_efs() + _logger.info( + "Number of projects that are currently in the EFS file system: %s", + len(efs_project_ids), + ) projects_repo = ProjectsRepo(app.state.engine) for project_id in efs_project_ids: diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks_setup.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks_setup.py index 81920ab23103..5874de42b0ce 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks_setup.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks_setup.py @@ -14,9 +14,9 @@ _logger = logging.getLogger(__name__) -_SEC = 1000 # in ms -_MIN = 60 * _SEC # in ms -_HOUR = 60 * _MIN # in ms +_SEC = 1 # in s +_MIN = 60 * _SEC # in s +_HOUR = 60 * _MIN # in s class EfsGuardianBackgroundTask(TypedDict): diff --git a/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages_setup.py b/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages_setup.py index d879d1891577..deb151fbda5d 100644 --- a/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages_setup.py +++ b/services/efs-guardian/src/simcore_service_efs_guardian/services/process_messages_setup.py @@ -39,7 +39,7 @@ async def _subscribe_to_rabbitmq(app) -> str: def _on_app_startup(app: FastAPI) -> Callable[[], Awaitable[None]]: async def _startup() -> None: with log_context( - _logger, logging.INFO, msg="setup resource tracker" + _logger, logging.INFO, msg="setup efs guardian process messages" ), log_catch(_logger, reraise=False): app_settings: ApplicationSettings = app.state.settings app.state.efs_guardian_rabbitmq_consumer = None From 1bbb6a988867ca9e444c74c0bdf1e207c4762112 Mon Sep 17 00:00:00 2001 From: matusdrobuliak66 Date: Wed, 23 Oct 2024 10:35:15 +0200 Subject: [PATCH 2/7] remove test --- packages/models-library/tests/test_projects_state.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/models-library/tests/test_projects_state.py b/packages/models-library/tests/test_projects_state.py index 2895d71f3a1b..475f3c8b8ff5 100644 --- a/packages/models-library/tests/test_projects_state.py +++ b/packages/models-library/tests/test_projects_state.py @@ -2,12 +2,6 @@ from models_library.projects_state import ProjectLocked, ProjectStatus -def test_project_locked_with_missing_owner_raises(): - with pytest.raises(ValueError): - ProjectLocked(value=True, status=ProjectStatus.OPENED) - ProjectLocked.parse_obj({"value": False, "status": ProjectStatus.OPENED}) - - @pytest.mark.parametrize( "lock, status", [ From 649b05b92399d6ac6c2bdaa7edcaa770ee84f5d0 Mon Sep 17 00:00:00 2001 From: matusdrobuliak66 Date: Wed, 23 Oct 2024 13:43:16 +0200 Subject: [PATCH 3/7] review @pcrespov --- .../src/models_library/projects_state.py | 19 ++++++++++++++++++- .../tests/test_projects_state.py | 12 +++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/models-library/src/models_library/projects_state.py b/packages/models-library/src/models_library/projects_state.py index 07b774805a09..9218639c74c1 100644 --- a/packages/models-library/src/models_library/projects_state.py +++ b/packages/models-library/src/models_library/projects_state.py @@ -57,10 +57,10 @@ class ProjectStatus(str, Enum): class ProjectLocked(BaseModel): value: bool = Field(..., description="True if the project is locked") + status: ProjectStatus = Field(..., description="The status of the project") owner: Owner | None = Field( default=None, description="If locked, the user that owns the lock" ) - status: ProjectStatus = Field(..., description="The status of the project") class Config: extra = Extra.forbid @@ -91,6 +91,23 @@ def check_status_compatible(cls, v, values): raise ValueError(msg) return v + @validator("owner", always=True) + @classmethod + def check_owner_compatible(cls, v, values): + if ( + values["value"] is True + and v is None + and values.get("status") + in [ + status.value + for status in ProjectStatus + if status != ProjectStatus.MAINTAINING + ] + ): + msg = "Owner must be specified when the project is not in the 'MAINTAINING' status." + raise ValueError(msg) + return v + class ProjectRunningState(BaseModel): value: RunningState = Field( diff --git a/packages/models-library/tests/test_projects_state.py b/packages/models-library/tests/test_projects_state.py index 475f3c8b8ff5..8a578c5d0f41 100644 --- a/packages/models-library/tests/test_projects_state.py +++ b/packages/models-library/tests/test_projects_state.py @@ -2,6 +2,16 @@ from models_library.projects_state import ProjectLocked, ProjectStatus +def test_project_locked_with_missing_owner_raises(): + with pytest.raises(ValueError): + ProjectLocked(value=True, status=ProjectStatus.OPENED) + ProjectLocked.parse_obj({"value": False, "status": ProjectStatus.OPENED}) + + +def test_project_locked_with_missing_owner_ok_during_maintaining(): + ProjectLocked.parse_obj({"value": True, "status": ProjectStatus.MAINTAINING}) + + @pytest.mark.parametrize( "lock, status", [ @@ -12,5 +22,5 @@ + [(True, ProjectStatus.CLOSED)], ) def test_project_locked_with_allowed_values(lock: bool, status: ProjectStatus): - with pytest.raises(ValueError): + with pytest.raises(ValueError) as e: ProjectLocked.parse_obj({"value": lock, "status": status}) From 9e53c35986bbdcc86bcbd022f64211c1e5f40933 Mon Sep 17 00:00:00 2001 From: matusdrobuliak66 Date: Wed, 23 Oct 2024 13:44:44 +0200 Subject: [PATCH 4/7] .. --- packages/models-library/src/models_library/projects_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/models-library/src/models_library/projects_state.py b/packages/models-library/src/models_library/projects_state.py index 9218639c74c1..110eddd60dc2 100644 --- a/packages/models-library/src/models_library/projects_state.py +++ b/packages/models-library/src/models_library/projects_state.py @@ -57,10 +57,10 @@ class ProjectStatus(str, Enum): class ProjectLocked(BaseModel): value: bool = Field(..., description="True if the project is locked") - status: ProjectStatus = Field(..., description="The status of the project") owner: Owner | None = Field( default=None, description="If locked, the user that owns the lock" ) + status: ProjectStatus = Field(..., description="The status of the project") class Config: extra = Extra.forbid From 6208df0a0236035c097e46a86b710851b4abcf9f Mon Sep 17 00:00:00 2001 From: matusdrobuliak66 Date: Wed, 23 Oct 2024 13:46:07 +0200 Subject: [PATCH 5/7] .. --- packages/models-library/tests/test_projects_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/models-library/tests/test_projects_state.py b/packages/models-library/tests/test_projects_state.py index 8a578c5d0f41..3f102de0436a 100644 --- a/packages/models-library/tests/test_projects_state.py +++ b/packages/models-library/tests/test_projects_state.py @@ -22,5 +22,5 @@ def test_project_locked_with_missing_owner_ok_during_maintaining(): + [(True, ProjectStatus.CLOSED)], ) def test_project_locked_with_allowed_values(lock: bool, status: ProjectStatus): - with pytest.raises(ValueError) as e: + with pytest.raises(ValueError): ProjectLocked.parse_obj({"value": lock, "status": status}) From 87057f6b69b1771a90a785feb15bf0a946eed339 Mon Sep 17 00:00:00 2001 From: matusdrobuliak66 Date: Wed, 23 Oct 2024 14:17:24 +0200 Subject: [PATCH 6/7] fix ordering --- packages/models-library/src/models_library/projects_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/models-library/src/models_library/projects_state.py b/packages/models-library/src/models_library/projects_state.py index 110eddd60dc2..9218639c74c1 100644 --- a/packages/models-library/src/models_library/projects_state.py +++ b/packages/models-library/src/models_library/projects_state.py @@ -57,10 +57,10 @@ class ProjectStatus(str, Enum): class ProjectLocked(BaseModel): value: bool = Field(..., description="True if the project is locked") + status: ProjectStatus = Field(..., description="The status of the project") owner: Owner | None = Field( default=None, description="If locked, the user that owns the lock" ) - status: ProjectStatus = Field(..., description="The status of the project") class Config: extra = Extra.forbid From aab352eb05596bb9459f0463e0f56af2802ce057 Mon Sep 17 00:00:00 2001 From: matusdrobuliak66 Date: Thu, 24 Oct 2024 10:15:14 +0200 Subject: [PATCH 7/7] review @sanderegg --- .../src/models_library/projects_state.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/models-library/src/models_library/projects_state.py b/packages/models-library/src/models_library/projects_state.py index 9218639c74c1..f9d9bf269839 100644 --- a/packages/models-library/src/models_library/projects_state.py +++ b/packages/models-library/src/models_library/projects_state.py @@ -5,7 +5,7 @@ from enum import Enum, unique from typing import Any, ClassVar -from pydantic import BaseModel, Extra, Field, validator +from pydantic import BaseModel, Extra, Field, root_validator, validator from .projects_access import Owner @@ -91,13 +91,13 @@ def check_status_compatible(cls, v, values): raise ValueError(msg) return v - @validator("owner", always=True) + @root_validator(pre=True) @classmethod - def check_owner_compatible(cls, v, values): + def check_owner_compatible(cls, values): if ( values["value"] is True - and v is None - and values.get("status") + and values.get("owner") is None + and values["status"] in [ status.value for status in ProjectStatus @@ -106,7 +106,7 @@ def check_owner_compatible(cls, v, values): ): msg = "Owner must be specified when the project is not in the 'MAINTAINING' status." raise ValueError(msg) - return v + return values class ProjectRunningState(BaseModel):