From e96b27b66ed5ac9d4eaf0fd8b20f756fb6177e17 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Fri, 10 Jun 2022 17:06:06 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Maintenance:=20Removed=20d?= =?UTF-8?q?eprecation=20warnings=20and=20marks=20flaky=20tests=20(#3085)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/models_library/settings/__init__.py | 3 -- .../models_library/settings/http_clients.py | 40 ------------------- .../postgres-database/requirements/_test.txt | 2 +- .../service-library/requirements/_test.in | 17 ++++---- .../service-library/requirements/_test.txt | 4 +- .../service-library/tests/test_async_utils.py | 9 ++++- packages/simcore-sdk/VERSION | 2 +- packages/simcore-sdk/setup.cfg | 2 +- .../src/simcore_sdk/config/http_clients.py | 2 +- .../client_session_manager.py | 4 +- .../simcore_service_catalog/core/events.py | 6 +-- .../services/director.py | 39 +++++++++++------- services/director-v2/requirements/_test.in | 37 ++++++----------- services/director-v2/requirements/_test.txt | 2 +- services/director-v2/setup.cfg | 3 ++ services/dynamic-sidecar/setup.cfg | 3 ++ .../tests/unit/test_api_containers.py | 1 - .../tests/unit/test_modules_mounted_fs.py | 9 +---- services/web/server/requirements/_test.txt | 2 +- 19 files changed, 72 insertions(+), 115 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/postgres-database/requirements/_test.txt b/packages/postgres-database/requirements/_test.txt index 6231c2986a0..a70b9169eb1 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.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 6b226054b33..f4ab7070542 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 @@ -113,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/packages/service-library/tests/test_async_utils.py b/packages/service-library/tests/test_async_utils.py index bb0d77ec352..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, @@ -155,7 +161,6 @@ 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) @@ -168,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( 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 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() 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 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 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 diff --git a/services/director-v2/requirements/_test.txt b/services/director-v2/requirements/_test.txt index e4e847acb00..1905a458a77 100644 --- a/services/director-v2/requirements/_test.txt +++ b/services/director-v2/requirements/_test.txt @@ -206,7 +206,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/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 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 diff --git a/services/web/server/requirements/_test.txt b/services/web/server/requirements/_test.txt index f190f7525d3..23a2bf1488e 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