From 85bfa3615664b0a4e34c3eb4422a5f516c689168 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Wed, 9 Apr 2025 08:57:15 +0200 Subject: [PATCH 1/4] the fix --- .../modules/auto_scaling_core.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/services/autoscaling/src/simcore_service_autoscaling/modules/auto_scaling_core.py b/services/autoscaling/src/simcore_service_autoscaling/modules/auto_scaling_core.py index 48567f3f0b60..0e8c0479ecf4 100644 --- a/services/autoscaling/src/simcore_service_autoscaling/modules/auto_scaling_core.py +++ b/services/autoscaling/src/simcore_service_autoscaling/modules/auto_scaling_core.py @@ -338,10 +338,10 @@ async def _sorted_allowed_instance_types(app: FastAPI) -> list[EC2InstanceType]: allowed_instance_type_names ), "EC2_INSTANCES_ALLOWED_TYPES cannot be empty!" - allowed_instance_types: list[ - EC2InstanceType - ] = await ec2_client.get_ec2_instance_capabilities( - cast(set[InstanceTypeType], set(allowed_instance_type_names)) + allowed_instance_types: list[EC2InstanceType] = ( + await ec2_client.get_ec2_instance_capabilities( + cast(set[InstanceTypeType], set(allowed_instance_type_names)) + ) ) def _as_selection(instance_type: EC2InstanceType) -> int: @@ -470,14 +470,14 @@ async def _start_warm_buffer_instances( with log_context( _logger, logging.INFO, f"start {len(instances_to_start)} buffer machines" ): + started_instances = await get_ec2_client(app).start_instances( + instances_to_start + ) + # NOTE: first start the instance and then set the tags in case the instance cannot start (e.g. InsufficientInstanceCapacity) await get_ec2_client(app).set_instances_tags( instances_to_start, tags=get_activated_buffer_ec2_tags(app, auto_scaling_mode), ) - - started_instances = await get_ec2_client(app).start_instances( - instances_to_start - ) started_instance_ids = [i.id for i in started_instances] return dataclasses.replace( @@ -669,7 +669,7 @@ async def _find_needed_instances( "found following %s needed instances: %s", len(needed_new_instance_types_for_tasks), [ - f"{i.instance_type.name}:{i.instance_type.resources} takes {len(i.assigned_tasks)} task{'s' if len(i.assigned_tasks)>1 else ''}" + f"{i.instance_type.name}:{i.instance_type.resources} takes {len(i.assigned_tasks)} task{'s' if len(i.assigned_tasks) > 1 else ''}" for i in needed_new_instance_types_for_tasks ], ) From 32876637a93182da5a0d80399d30e774757b71fe Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Wed, 9 Apr 2025 09:37:37 +0200 Subject: [PATCH 2/4] clean --- packages/aws-library/src/aws_library/ec2/_error_handler.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/aws-library/src/aws_library/ec2/_error_handler.py b/packages/aws-library/src/aws_library/ec2/_error_handler.py index fc22447caa07..8984cf6a0a36 100644 --- a/packages/aws-library/src/aws_library/ec2/_error_handler.py +++ b/packages/aws-library/src/aws_library/ec2/_error_handler.py @@ -59,13 +59,8 @@ def ec2_exception_handler( [Callable[Concatenate[Self, P], Coroutine[Any, Any, R]]], Callable[Concatenate[Self, P], Coroutine[Any, Any, R]], ]: - """ - Raises: - SSMAccessError: - """ - def decorator( - func: Callable[Concatenate[Self, P], Coroutine[Any, Any, R]] + func: Callable[Concatenate[Self, P], Coroutine[Any, Any, R]], ) -> Callable[Concatenate[Self, P], Coroutine[Any, Any, R]]: @functools.wraps(func) async def wrapper(self: Self, *args: P.args, **kwargs: P.kwargs) -> R: From fab4bd6c3ff2ce698b2d6c421e29b392baeb3cd2 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Wed, 9 Apr 2025 09:45:38 +0200 Subject: [PATCH 3/4] fixes invisible issues --- .../src/servicelib/background_task.py | 5 +++-- .../tests/test_background_task.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/service-library/src/servicelib/background_task.py b/packages/service-library/src/servicelib/background_task.py index feeb06ff4753..508f34b99eec 100644 --- a/packages/service-library/src/servicelib/background_task.py +++ b/packages/service-library/src/servicelib/background_task.py @@ -10,7 +10,7 @@ from tenacity.wait import wait_fixed from .async_utils import cancel_wait_task, delayed_start -from .logging_utils import log_context +from .logging_utils import log_catch, log_context _logger = logging.getLogger(__name__) @@ -84,7 +84,8 @@ class _InternalTryAgain(TryAgain): ) @functools.wraps(func) async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> None: - await func(*args, **kwargs) + with log_catch(_logger, reraise=True): + await func(*args, **kwargs) raise _InternalTryAgain return _wrapper diff --git a/packages/service-library/tests/test_background_task.py b/packages/service-library/tests/test_background_task.py index 0f23035318fa..8c508bf8979c 100644 --- a/packages/service-library/tests/test_background_task.py +++ b/packages/service-library/tests/test_background_task.py @@ -6,6 +6,7 @@ import asyncio import datetime +import logging from collections.abc import AsyncIterator, Awaitable, Callable from typing import Final from unittest import mock @@ -204,3 +205,19 @@ async def _func() -> None: await task assert mock_func.call_count > 1 + + +async def test_periodic_task_logs_error( + mock_background_task: mock.AsyncMock, + task_interval: datetime.timedelta, + caplog: pytest.LogCaptureFixture, +): + mock_background_task.side_effect = RuntimeError("Test error") + + with caplog.at_level(logging.ERROR): + async with periodic_task( + mock_background_task, interval=task_interval, task_name="test_task" + ): + await asyncio.sleep(2 * task_interval.total_seconds()) + + assert "Test error" in caplog.text From 1aa81fb82cefda23587df369bdd288cd0d2b82de Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Wed, 9 Apr 2025 22:10:36 +0200 Subject: [PATCH 4/4] @pcrespov review: good catch --- .../simcore_service_autoscaling/modules/auto_scaling_core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/autoscaling/src/simcore_service_autoscaling/modules/auto_scaling_core.py b/services/autoscaling/src/simcore_service_autoscaling/modules/auto_scaling_core.py index 0e8c0479ecf4..1525aa9eee3c 100644 --- a/services/autoscaling/src/simcore_service_autoscaling/modules/auto_scaling_core.py +++ b/services/autoscaling/src/simcore_service_autoscaling/modules/auto_scaling_core.py @@ -475,7 +475,7 @@ async def _start_warm_buffer_instances( ) # NOTE: first start the instance and then set the tags in case the instance cannot start (e.g. InsufficientInstanceCapacity) await get_ec2_client(app).set_instances_tags( - instances_to_start, + started_instances, tags=get_activated_buffer_ec2_tags(app, auto_scaling_mode), ) started_instance_ids = [i.id for i in started_instances]