Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ Maintenance: Removed deprecation warnings and marks flaky tests #3085

Merged
merged 13 commits into from
Jun 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

This file was deleted.

2 changes: 1 addition & 1 deletion packages/postgres-database/requirements/_test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 7 additions & 10 deletions packages/service-library/requirements/_test.in
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 3 additions & 1 deletion packages/service-library/requirements/_test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions packages/service-library/tests/test_async_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion packages/simcore-sdk/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.3.0
1.3.1
2 changes: 1 addition & 1 deletion packages/simcore-sdk/setup.cfg
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
from models_library.settings.http_clients import ClientRequestSettings
from settings_library.http_client_request import ClientRequestSettings

client_request_settings = ClientRequestSettings()
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions services/catalog/src/simcore_service_catalog/core/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
39 changes: 24 additions & 15 deletions services/catalog/src/simcore_service_catalog/services/director.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the point of declaring this variable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this way we can get autocompletion, search, etc in vscode

if client := app.state.director_api:
await client.close()

logger.debug("Director client closed successfully")

Expand Down Expand Up @@ -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
Expand Down
37 changes: 13 additions & 24 deletions services/director-v2/requirements/_test.in
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
2 changes: 1 addition & 1 deletion services/director-v2/requirements/_test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions services/director-v2/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ tag = False
commit_args = --no-verify

[bumpversion:file:VERSION]

[tool:pytest]
asyncio_mode = auto
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this necessary? it is already in the makefile right?

Copy link
Member Author

@pcrespov pcrespov Jun 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it? I need to double check then. There was a warning in the tests ...

../../.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)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yes, I see that the scripts in ci and the makefile recipes incorporate this option in CLI
That is strange then. Perhaps I was running pytest manually via CLI.

3 changes: 3 additions & 0 deletions services/dynamic-sidecar/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ tag = False
commit_args = --no-verify

[bumpversion:file:VERSION]

[tool:pytest]
asyncio_mode = auto
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
DEFAULT_COMMAND_TIMEOUT = 5.0
WAIT_FOR_DIRECTORY_WATCHER = 0.1

pytestmark = pytest.mark.asyncio

# FIXTURES

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,18 @@

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


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


Expand Down
2 changes: 1 addition & 1 deletion services/web/server/requirements/_test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down