From 61442a5362edfdb5df23dc0edec5a93e8986339d Mon Sep 17 00:00:00 2001 From: Pedro Crespo <32402063+pcrespov@users.noreply.github.com> Date: Thu, 2 Jun 2022 11:32:52 +0200 Subject: [PATCH 01/10] flaky test on servicelib -> ANE? --- packages/service-library/requirements/_test.in | 17 +++++++---------- packages/service-library/requirements/_test.txt | 2 ++ .../service-library/tests/test_async_utils.py | 1 + 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/service-library/requirements/_test.in b/packages/service-library/requirements/_test.in index 7d462bd1491..cfcf5566599 100644 --- a/packages/service-library/requirements/_test.in +++ b/packages/service-library/requirements/_test.in @@ -12,19 +12,16 @@ # testing coverage +coveralls +faker +flaky +openapi-spec-validator +pylint # NOTE: The version in pylint at _text.txt is used as a reference for ci/helpers/install_pylint.bash pytest pytest-aiohttp # incompatible with pytest-asyncio. See https://github.com/pytest-dev/pytest-asyncio/issues/76 pytest-cov -pytest-instafail -pytest-runner pytest-docker +pytest-instafail pytest-mock +pytest-runner pytest-sugar - -faker - -pylint # NOTE: The version in pylint at _text.txt is used as a reference for ci/helpers/install_pylint.bash -coveralls - -# openapi validator -openapi-spec-validator diff --git a/packages/service-library/requirements/_test.txt b/packages/service-library/requirements/_test.txt index 1ca60853c8d..14c93c9166a 100644 --- a/packages/service-library/requirements/_test.txt +++ b/packages/service-library/requirements/_test.txt @@ -69,6 +69,8 @@ docopt==0.6.2 # docker-compose faker==13.7.0 # via -r requirements/_test.in +flaky==3.7.0 + # via -r requirements/_test.in frozenlist==1.3.0 # via # -c requirements/_aiohttp.txt diff --git a/packages/service-library/tests/test_async_utils.py b/packages/service-library/tests/test_async_utils.py index bb0d77ec352..4ff694f58ea 100644 --- a/packages/service-library/tests/test_async_utils.py +++ b/packages/service-library/tests/test_async_utils.py @@ -151,6 +151,7 @@ async def target_function(the_param: Any) -> None: assert str(excinfo.value).startswith(message) is True +@pytest.mark.flaky(max_runs=3) # FIXME: ANE pls review and remove this flaky mark async def test_context_aware_measure_parallelism( sleep_duration: float, ensure_run_in_sequence_context_is_empty: None, From 9016616a220385578d98f6f3f2923e9013a279b8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo <32402063+pcrespov@users.noreply.github.com> Date: Thu, 2 Jun 2022 11:41:10 +0200 Subject: [PATCH 02/10] fixes deprecation warning for asyncio_mode ../../.venv/lib/python3.9/site-packages/pytest_aiohttp/plugin.py:28 /home/crespo/devp/osparc-simcore/.venv/lib/python3.9/site-packages/pytest_aiohttp/plugin.py:28: DeprecationWarning: The 'asyncio_mode' is 'legacy', switching to 'auto' for the sake of pytest-aiohttp backward compatibility. Please explicitly use 'asyncio_mode=strict' or 'asyncio_mode=auto' in pytest configuration file. config.issue_config_time_warning(LEGACY_MODE, stacklevel=2) --- services/director-v2/setup.cfg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/director-v2/setup.cfg b/services/director-v2/setup.cfg index f79f8b9dcd4..516e1deaaec 100644 --- a/services/director-v2/setup.cfg +++ b/services/director-v2/setup.cfg @@ -6,3 +6,6 @@ tag = False commit_args = --no-verify [bumpversion:file:VERSION] + +[tool:pytest] +asyncio_mode = auto From f0089ffcfdd82a53c3f98e6be31fa7178575e862 Mon Sep 17 00:00:00 2001 From: Pedro Crespo <32402063+pcrespov@users.noreply.github.com> Date: Thu, 2 Jun 2022 11:43:27 +0200 Subject: [PATCH 03/10] minor --- services/director-v2/requirements/_test.in | 37 ++++++++-------------- 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/services/director-v2/requirements/_test.in b/services/director-v2/requirements/_test.in index 3f6760dc459..48aa18befb0 100644 --- a/services/director-v2/requirements/_test.in +++ b/services/director-v2/requirements/_test.in @@ -9,9 +9,20 @@ # --constraint _base.txt - -# testing +aio_pika +aioboto3 +alembic # migration due to pytest_simcore.postgres_service2 +asgi_lifespan +async-asgi-testclient +bokeh +codecov +coveralls +dask-gateway-server[local] +docker +Faker flaky +minio +pylint pytest pytest-aiohttp pytest-cov @@ -20,26 +31,4 @@ pytest-icdiff pytest-mock pytest-runner pytest-xdist - -# fixtures -asgi_lifespan -Faker - -# helpers -aioboto3 -async-asgi-testclient -minio - - -# migration due to pytest_simcore.postgres_service2 -aio_pika -alembic -bokeh -dask-gateway-server[local] -docker respx - -# tools -codecov -coveralls -pylint From 1c5615ab860642a4965930ffa5b4de7a7c8584c6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo <32402063+pcrespov@users.noreply.github.com> Date: Wed, 8 Jun 2022 18:17:39 +0200 Subject: [PATCH 04/10] rm DeprecationWarning: models_library.settings.http_clients is deprecated, use instead settings_library.http_client_request --- .../src/models_library/settings/__init__.py | 3 -- .../models_library/settings/http_clients.py | 40 ------------------- .../src/simcore_sdk/config/http_clients.py | 2 +- 3 files changed, 1 insertion(+), 44 deletions(-) delete mode 100644 packages/models-library/src/models_library/settings/__init__.py delete mode 100644 packages/models-library/src/models_library/settings/http_clients.py diff --git a/packages/models-library/src/models_library/settings/__init__.py b/packages/models-library/src/models_library/settings/__init__.py deleted file mode 100644 index 8b552a24cac..00000000000 --- a/packages/models-library/src/models_library/settings/__init__.py +++ /dev/null @@ -1,3 +0,0 @@ -""" - Common settings -""" diff --git a/packages/models-library/src/models_library/settings/http_clients.py b/packages/models-library/src/models_library/settings/http_clients.py deleted file mode 100644 index eb96b143ca0..00000000000 --- a/packages/models-library/src/models_library/settings/http_clients.py +++ /dev/null @@ -1,40 +0,0 @@ -import warnings -from typing import Optional - -from pydantic import BaseSettings, Field - -warnings.warn( - f"{__name__} is deprecated, use instead settings_library.http_client_request", - DeprecationWarning, -) - - -class ClientRequestSettings(BaseSettings): - # NOTE: These entries are used in some old services as well. These need to be updated if these - # variable names or defaults are changed. - total_timeout: Optional[int] = Field( - default=20, - description="timeout in seconds used for outgoing http requests", - env="HTTP_CLIENT_REQUEST_TOTAL_TIMEOUT", - ) - - aiohttp_connect_timeout: Optional[int] = Field( - default=None, - description=( - "Maximal number of seconds for acquiring a connection" - " from pool. The time consists connection establishment" - " for a new connection or waiting for a free connection" - " from a pool if pool connection limits are exceeded. " - "For pure socket connection establishment time use sock_connect." - ), - env="HTTP_CLIENT_REQUEST_AIOHTTP_CONNECT_TIMEOUT", - ) - - aiohttp_sock_connect_timeout: Optional[int] = Field( - default=5, - description=( - "aiohttp specific field used in ClientTimeout, timeout for connecting to a " - "peer for a new connection not given a pool" - ), - env="HTTP_CLIENT_REQUEST_AIOHTTP_SOCK_CONNECT_TIMEOUT", - ) diff --git a/packages/simcore-sdk/src/simcore_sdk/config/http_clients.py b/packages/simcore-sdk/src/simcore_sdk/config/http_clients.py index 83049c10b3e..2f34252f33b 100644 --- a/packages/simcore-sdk/src/simcore_sdk/config/http_clients.py +++ b/packages/simcore-sdk/src/simcore_sdk/config/http_clients.py @@ -1,3 +1,3 @@ -from models_library.settings.http_clients import ClientRequestSettings +from settings_library.http_client_request import ClientRequestSettings client_request_settings = ClientRequestSettings() From b66a803b4087023a8e7737202b12ec8dea1c52c7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo <32402063+pcrespov@users.noreply.github.com> Date: Wed, 8 Jun 2022 18:35:14 +0200 Subject: [PATCH 05/10] Fixes _client.py:2012: UserWarning: Unclosed . See https://www.python-httpx.org/async/\#opening-and-closing-clients for details. --- .../simcore_service_catalog/core/events.py | 6 +-- .../services/director.py | 39 ++++++++++++------- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index 04992589404..931889deddc 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -50,10 +50,10 @@ async def start_app() -> None: if app.state.settings.CATALOG_POSTGRES: await connect_to_db(app) - # setup connection to director - await setup_director(app) - if app.state.settings.CATALOG_DIRECTOR: + # setup connection to director + await setup_director(app) + # FIXME: check director service is in place and ready. Hand-shake?? # SEE https://github.com/ITISFoundation/osparc-simcore/issues/1728 await start_registry_sync_task(app) diff --git a/services/catalog/src/simcore_service_catalog/services/director.py b/services/catalog/src/simcore_service_catalog/services/director.py index 040abecb2a9..a2263a09d96 100644 --- a/services/catalog/src/simcore_service_catalog/services/director.py +++ b/services/catalog/src/simcore_service_catalog/services/director.py @@ -1,7 +1,7 @@ import asyncio import functools import logging -from typing import Any, Awaitable, Callable, Dict, List, Union +from typing import Any, Awaitable, Callable, Dict, List, Optional, Union import httpx from fastapi import FastAPI, HTTPException @@ -27,29 +27,38 @@ ) +class UnresponsiveService(RuntimeError): + pass + + async def setup_director(app: FastAPI) -> None: if settings := app.state.settings.CATALOG_DIRECTOR: # init client-api - logger.debug("Setup director at %s...", settings.base_url) - director_client = DirectorApi(base_url=settings.base_url, app=app) + logger.debug("Setup director at %s ...", f"{settings.base_url=}") + client = DirectorApi(base_url=settings.base_url, app=app) # check that the director is accessible - async for attempt in AsyncRetrying(**director_startup_retry_policy): - with attempt: - if not await director_client.is_responsive(): - raise ValueError("Director-v0 is not responsive") + try: + async for attempt in AsyncRetrying(**director_startup_retry_policy): + with attempt: + if not await client.is_responsive(): + raise UnresponsiveService("Director-v0 is not responsive") - logger.info( - "Connection to director-v0 succeded [%s]", - json_dumps(attempt.retry_state.retry_object.statistics), - ) + logger.info( + "Connection to director-v0 succeded [%s]", + json_dumps(attempt.retry_state.retry_object.statistics), + ) + except UnresponsiveService: + await client.close() + raise - app.state.director_api = director_client + app.state.director_api = client async def close_director(app: FastAPI) -> None: - if director_client := app.state.director_api: - await director_client.delete() + client: Optional[DirectorApi] + if client := app.state.director_api: + await client.close() logger.debug("Director client closed successfully") @@ -131,7 +140,7 @@ def __init__(self, base_url: str, app: FastAPI): ) self.vtag = app.state.settings.CATALOG_DIRECTOR.DIRECTOR_VTAG - async def delete(self): + async def close(self): await self.client.aclose() # OPERATIONS From b500bbe0e769b32be947e371450a573f98a70ae6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo <32402063+pcrespov@users.noreply.github.com> Date: Wed, 8 Jun 2022 18:40:56 +0200 Subject: [PATCH 06/10] Fixes PytestWarning: The test is marked with '@pytest.mark.asyncio' but it is not an async function. Please remove asyncio marker. If the test is not marked explicitly, check for global markers applied via 'pytestmark'. --- services/dynamic-sidecar/setup.cfg | 3 +++ .../dynamic-sidecar/tests/unit/test_api_containers.py | 1 - .../tests/unit/test_modules_mounted_fs.py | 9 +-------- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/services/dynamic-sidecar/setup.cfg b/services/dynamic-sidecar/setup.cfg index 3d704e33062..f2beddcb7ed 100644 --- a/services/dynamic-sidecar/setup.cfg +++ b/services/dynamic-sidecar/setup.cfg @@ -6,3 +6,6 @@ tag = False commit_args = --no-verify [bumpversion:file:VERSION] + +[tool:pytest] +asyncio_mode = auto diff --git a/services/dynamic-sidecar/tests/unit/test_api_containers.py b/services/dynamic-sidecar/tests/unit/test_api_containers.py index c8f850a7183..ee123f9d567 100644 --- a/services/dynamic-sidecar/tests/unit/test_api_containers.py +++ b/services/dynamic-sidecar/tests/unit/test_api_containers.py @@ -36,7 +36,6 @@ DEFAULT_COMMAND_TIMEOUT = 5.0 WAIT_FOR_DIRECTORY_WATCHER = 0.1 -pytestmark = pytest.mark.asyncio # FIXTURES diff --git a/services/dynamic-sidecar/tests/unit/test_modules_mounted_fs.py b/services/dynamic-sidecar/tests/unit/test_modules_mounted_fs.py index b160d3fa59f..fe605367977 100644 --- a/services/dynamic-sidecar/tests/unit/test_modules_mounted_fs.py +++ b/services/dynamic-sidecar/tests/unit/test_modules_mounted_fs.py @@ -4,13 +4,11 @@ import os from pathlib import Path -from typing import Any, List +from typing import List import pytest from simcore_service_dynamic_sidecar.modules import mounted_fs -pytestmark = pytest.mark.asyncio - # UTILS @@ -18,11 +16,6 @@ def _replace_slashes(path: Path) -> str: return str(path).replace(os.sep, "_") -def _assert_same_object(first: Any, second: Any) -> None: - assert first == second - assert id(first) == id(second) - - # FIXTURES From 9ddf912ac3477c2c97a1c8c8e7071bc945208014 Mon Sep 17 00:00:00 2001 From: Pedro Crespo <32402063+pcrespov@users.noreply.github.com> Date: Wed, 8 Jun 2022 18:50:49 +0200 Subject: [PATCH 07/10] fix --- .../simcore_sdk/node_ports_common/client_session_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/simcore-sdk/src/simcore_sdk/node_ports_common/client_session_manager.py b/packages/simcore-sdk/src/simcore_sdk/node_ports_common/client_session_manager.py index e9934789279..323ae7e6b58 100644 --- a/packages/simcore-sdk/src/simcore_sdk/node_ports_common/client_session_manager.py +++ b/packages/simcore-sdk/src/simcore_sdk/node_ports_common/client_session_manager.py @@ -19,8 +19,8 @@ def __init__(self, session=None): self.active_session = session or ClientSession( timeout=ClientTimeout( total=None, - connect=client_request_settings.aiohttp_connect_timeout, - sock_connect=client_request_settings.aiohttp_sock_connect_timeout, + connect=client_request_settings.HTTP_CLIENT_REQUEST_AIOHTTP_CONNECT_TIMEOUT, + sock_connect=client_request_settings.HTTP_CLIENT_REQUEST_AIOHTTP_SOCK_CONNECT_TIMEOUT, ) # type: ignore ) self.is_owned = self.active_session is not session From c2d98733b61c33d92eb4c53da43a6f1de1ec3255 Mon Sep 17 00:00:00 2001 From: Pedro Crespo <32402063+pcrespov@users.noreply.github.com> Date: Wed, 8 Jun 2022 18:51:16 +0200 Subject: [PATCH 08/10] =?UTF-8?q?packages/simcore-sdk=20version:=201.3.0?= =?UTF-8?q?=20=E2=86=92=201.3.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/simcore-sdk/VERSION | 2 +- packages/simcore-sdk/setup.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/simcore-sdk/VERSION b/packages/simcore-sdk/VERSION index f0bb29e7638..3a3cd8cc8b0 100644 --- a/packages/simcore-sdk/VERSION +++ b/packages/simcore-sdk/VERSION @@ -1 +1 @@ -1.3.0 +1.3.1 diff --git a/packages/simcore-sdk/setup.cfg b/packages/simcore-sdk/setup.cfg index 8788f2f316a..af375f03797 100644 --- a/packages/simcore-sdk/setup.cfg +++ b/packages/simcore-sdk/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 1.3.0 +current_version = 1.3.1 commit = True message = packages/simcore-sdk version: {current_version} → {new_version} tag = False From 1a97849cfcaffe1eb31bb593d971de0f5c010e22 Mon Sep 17 00:00:00 2001 From: Pedro Crespo <32402063+pcrespov@users.noreply.github.com> Date: Wed, 8 Jun 2022 21:12:21 +0200 Subject: [PATCH 09/10] upgrades paramiko to avoid Warnings due to Blowfish deprecation in cryptography 37.0.0 --- packages/postgres-database/requirements/_test.txt | 2 +- packages/service-library/requirements/_test.txt | 2 +- services/director-v2/requirements/_base.txt | 4 +++- services/director-v2/requirements/_test.txt | 2 +- services/web/server/requirements/_test.txt | 2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/postgres-database/requirements/_test.txt b/packages/postgres-database/requirements/_test.txt index b60a41a418b..a552bf38d19 100644 --- a/packages/postgres-database/requirements/_test.txt +++ b/packages/postgres-database/requirements/_test.txt @@ -103,7 +103,7 @@ multidict==6.0.2 # yarl packaging==21.3 # via pytest -paramiko==2.10.4 +paramiko==2.11.0 # via # -c requirements/../../../requirements/constraints.txt # docker diff --git a/packages/service-library/requirements/_test.txt b/packages/service-library/requirements/_test.txt index 14c93c9166a..262ef05fc32 100644 --- a/packages/service-library/requirements/_test.txt +++ b/packages/service-library/requirements/_test.txt @@ -115,7 +115,7 @@ packaging==21.3 # via # pytest # pytest-sugar -paramiko==2.10.4 +paramiko==2.11.0 # via # -c requirements/../../../requirements/constraints.txt # docker diff --git a/services/director-v2/requirements/_base.txt b/services/director-v2/requirements/_base.txt index 1fdfc9c4cac..ffb97717d3d 100644 --- a/services/director-v2/requirements/_base.txt +++ b/services/director-v2/requirements/_base.txt @@ -183,7 +183,9 @@ jsonschema==3.2.0 # -c requirements/../../../packages/service-library/requirements/././constraints.txt # -c requirements/../../../packages/service-library/requirements/./constraints.txt # -c requirements/../../../packages/simcore-sdk/requirements/../../../packages/service-library/requirements/./constraints.txt - # -r requirements/../../../packages/simcore-sdk/requirements/_base.in + # -r requirements/../../../packages/dask-task-models-library/requirements/../../../packages/models-library/requirements/_base.in + # -r requirements/../../../packages/models-library/requirements/_base.in + # -r requirements/../../../packages/simcore-sdk/requirements/../../../packages/models-library/requirements/_base.in locket==0.2.1 # via # -r requirements/../../../services/dask-sidecar/requirements/_dask-distributed.txt diff --git a/services/director-v2/requirements/_test.txt b/services/director-v2/requirements/_test.txt index d611653adda..6699ba33b2a 100644 --- a/services/director-v2/requirements/_test.txt +++ b/services/director-v2/requirements/_test.txt @@ -209,7 +209,7 @@ pamqp==2.3.0 # via # -c requirements/_base.txt # aiormq -paramiko==2.10.4 +paramiko==2.11.0 # via # -c requirements/../../../requirements/constraints.txt # docker diff --git a/services/web/server/requirements/_test.txt b/services/web/server/requirements/_test.txt index 47ea015e550..222b30aa1d8 100644 --- a/services/web/server/requirements/_test.txt +++ b/services/web/server/requirements/_test.txt @@ -157,7 +157,7 @@ packaging==21.3 # pytest # pytest-sugar # redis -paramiko==2.10.4 +paramiko==2.11.0 # via # -c requirements/../../../../requirements/constraints.txt # docker From a3a8a8a5a5a8028e7aee3f304ae23448b5d2e39d Mon Sep 17 00:00:00 2001 From: Andrei Neagu <5694077+GitHK@users.noreply.github.com> Date: Fri, 10 Jun 2022 14:58:53 +0200 Subject: [PATCH 10/10] Fixes flaky test (#68) * fixed test * refactor test Co-authored-by: Andrei Neagu --- packages/service-library/tests/test_async_utils.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/service-library/tests/test_async_utils.py b/packages/service-library/tests/test_async_utils.py index 4ff694f58ea..b8e11ddeac9 100644 --- a/packages/service-library/tests/test_async_utils.py +++ b/packages/service-library/tests/test_async_utils.py @@ -60,6 +60,12 @@ async def get_all(self) -> List[Any]: return list(self._queue) +def _compensate_for_slow_systems(number: float) -> float: + # NOTE: in slower systems it is important to allow for enough time to pass + # raising by one order of magnitude + return number * 10 + + async def test_context_aware_dispatch( sleep_duration: float, ensure_run_in_sequence_context_is_empty: None, @@ -151,12 +157,10 @@ async def target_function(the_param: Any) -> None: assert str(excinfo.value).startswith(message) is True -@pytest.mark.flaky(max_runs=3) # FIXME: ANE pls review and remove this flaky mark async def test_context_aware_measure_parallelism( sleep_duration: float, ensure_run_in_sequence_context_is_empty: None, ) -> None: - # expected duration 1 second @run_sequentially_in_context(target_args=["control"]) async def sleep_for(sleep_interval: float, control: Any) -> Any: await asyncio.sleep(sleep_interval) @@ -169,8 +173,8 @@ async def sleep_for(sleep_interval: float, control: Any) -> Any: result = await asyncio.gather(*functions) elapsed = time() - start - assert elapsed < sleep_duration * 2 # allow for some internal delay assert control_sequence == result + assert elapsed < _compensate_for_slow_systems(sleep_duration) async def test_context_aware_measure_serialization(