From 68109d5dbb27abafe5bec11a5d518a8c4cb5e077 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 31 Mar 2025 18:15:44 +0200 Subject: [PATCH 1/3] fix --- .../simcore_service_director/core/application.py | 1 + .../src/simcore_service_director/core/settings.py | 12 +++++++++++- services/director/tests/unit/test_core_settings.py | 13 +++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/services/director/src/simcore_service_director/core/application.py b/services/director/src/simcore_service_director/core/application.py index c6df150dde3..58fa66a98ef 100644 --- a/services/director/src/simcore_service_director/core/application.py +++ b/services/director/src/simcore_service_director/core/application.py @@ -56,6 +56,7 @@ def create_app(settings: ApplicationSettings) -> FastAPI: setup_client_session( app, max_keepalive_connections=settings.DIRECTOR_REGISTRY_CLIENT_MAX_KEEPALIVE_CONNECTIONS, + timeout=settings.DIRECTOR_REGISTRY_CLIENT_TIMEOUT, ) setup_registry(app) diff --git a/services/director/src/simcore_service_director/core/settings.py b/services/director/src/simcore_service_director/core/settings.py index bf949cb5827..c9502b4568d 100644 --- a/services/director/src/simcore_service_director/core/settings.py +++ b/services/director/src/simcore_service_director/core/settings.py @@ -112,10 +112,20 @@ class ApplicationSettings(BaseApplicationSettings, MixinLoggingSettings): ), ) - DIRECTOR_REGISTRY_CLIENT_MAX_KEEPALIVE_CONNECTIONS: NonNegativeInt = 0 + DIRECTOR_REGISTRY_CLIENT_MAX_KEEPALIVE_CONNECTIONS: NonNegativeInt = 5 + DIRECTOR_REGISTRY_CLIENT_TIMEOUT: datetime.timedelta = datetime.timedelta( + seconds=20 + ) DIRECTOR_REGISTRY_CLIENT_MAX_CONCURRENT_CALLS: PositiveInt = 20 DIRECTOR_REGISTRY_CLIENT_MAX_NUMBER_OF_RETRIEVED_OBJECTS: PositiveInt = 30 + @field_validator("DIRECTOR_REGISTRY_CLIENT_TIMEOUT") + @classmethod + def _check_positive(cls, value: datetime.timedelta) -> None: + if value.total_seconds() < 0: + msg = "DIRECTOR_REGISTRY_CLIENT_TIMEOUT must be positive" + raise ValueError(msg) + @field_validator("DIRECTOR_GENERIC_RESOURCE_PLACEMENT_CONSTRAINTS_SUBSTITUTIONS") @classmethod def _validate_substitutions(cls, v): diff --git a/services/director/tests/unit/test_core_settings.py b/services/director/tests/unit/test_core_settings.py index dfccd67ce2f..5e8b42c1268 100644 --- a/services/director/tests/unit/test_core_settings.py +++ b/services/director/tests/unit/test_core_settings.py @@ -4,7 +4,10 @@ # pylint: disable=too-many-arguments +import datetime + import pytest +from pydantic import ValidationError from pytest_simcore.helpers.monkeypatch_envs import ( setenvs_from_dict, setenvs_from_envfile, @@ -37,6 +40,16 @@ def test_valid_web_application_settings(app_environment: EnvVarsDict): ) +def test_invalid_client_timeout_raises( + app_environment: EnvVarsDict, monkeypatch: pytest.MonkeyPatch +): + monkeypatch.setenv( + "DIRECTOR_REGISTRY_CLIENT_TIMEOUT", f"{datetime.timedelta(seconds=-10)}" + ) + with pytest.raises(ValidationError): + ApplicationSettings.create_from_envs() + + def test_docker_container_env_sample(monkeypatch: pytest.MonkeyPatch): monkeypatch.delenv("DIRECTOR_DEFAULT_MAX_MEMORY", raising=False) From b171f5360dd98b809bcfe3e957bf5140151556f3 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 31 Mar 2025 18:21:54 +0200 Subject: [PATCH 2/3] sonar --- .../src/servicelib/fastapi/client_session.py | 10 +++++++++- .../src/simcore_service_director/core/application.py | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/client_session.py b/packages/service-library/src/servicelib/fastapi/client_session.py index c3bc2728c64..b92dcc2d525 100644 --- a/packages/service-library/src/servicelib/fastapi/client_session.py +++ b/packages/service-library/src/servicelib/fastapi/client_session.py @@ -1,12 +1,20 @@ +import datetime + import httpx from fastapi import FastAPI -def setup_client_session(app: FastAPI, *, max_keepalive_connections: int = 20) -> None: +def setup_client_session( + app: FastAPI, + *, + default_timeout: datetime.timedelta = datetime.timedelta(seconds=20), + max_keepalive_connections: int = 20 +) -> None: async def on_startup() -> None: session = httpx.AsyncClient( transport=httpx.AsyncHTTPTransport(http2=True), limits=httpx.Limits(max_keepalive_connections=max_keepalive_connections), + timeout=default_timeout.total_seconds(), ) app.state.aiohttp_client_session = session diff --git a/services/director/src/simcore_service_director/core/application.py b/services/director/src/simcore_service_director/core/application.py index 58fa66a98ef..24716583b92 100644 --- a/services/director/src/simcore_service_director/core/application.py +++ b/services/director/src/simcore_service_director/core/application.py @@ -56,7 +56,7 @@ def create_app(settings: ApplicationSettings) -> FastAPI: setup_client_session( app, max_keepalive_connections=settings.DIRECTOR_REGISTRY_CLIENT_MAX_KEEPALIVE_CONNECTIONS, - timeout=settings.DIRECTOR_REGISTRY_CLIENT_TIMEOUT, + default_timeout=settings.DIRECTOR_REGISTRY_CLIENT_TIMEOUT, ) setup_registry(app) From 50d1f48fa5d7556366be130d54842c24d54ffb18 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Mon, 31 Mar 2025 18:28:29 +0200 Subject: [PATCH 3/3] return value --- .../director/src/simcore_service_director/core/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/director/src/simcore_service_director/core/settings.py b/services/director/src/simcore_service_director/core/settings.py index c9502b4568d..5560de876fa 100644 --- a/services/director/src/simcore_service_director/core/settings.py +++ b/services/director/src/simcore_service_director/core/settings.py @@ -121,10 +121,11 @@ class ApplicationSettings(BaseApplicationSettings, MixinLoggingSettings): @field_validator("DIRECTOR_REGISTRY_CLIENT_TIMEOUT") @classmethod - def _check_positive(cls, value: datetime.timedelta) -> None: + def _check_positive(cls, value: datetime.timedelta) -> datetime.timedelta: if value.total_seconds() < 0: msg = "DIRECTOR_REGISTRY_CLIENT_TIMEOUT must be positive" raise ValueError(msg) + return value @field_validator("DIRECTOR_GENERIC_RESOURCE_PLACEMENT_CONSTRAINTS_SUBSTITUTIONS") @classmethod