Skip to content

BugFix: fixes awaitables and broad-exceptions in garbage-collector #2104

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

Merged
merged 9 commits into from
Feb 10, 2021
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
23 changes: 19 additions & 4 deletions packages/postgres-database/tests/test_groups.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
# pylint: disable=no-name-in-module
# pylint: disable=no-value-for-parameter

import pytest
from aiopg.sa.result import RowProxy
from psycopg2.errors import ForeignKeyViolation, RaiseException, UniqueViolation
from sqlalchemy import literal_column

from typing import Optional

import aiopg.sa.exc
import pytest
from aiopg.sa.result import ResultProxy, RowProxy
from fake_creators import random_group, random_user
from psycopg2.errors import ForeignKeyViolation, RaiseException, UniqueViolation
from simcore_postgres_database.models.base import metadata
from simcore_postgres_database.webserver_models import (
GroupType,
groups,
user_to_groups,
users,
)
from sqlalchemy import literal_column


async def _create_group(conn, **overrides) -> RowProxy:
Expand Down Expand Up @@ -41,6 +44,7 @@ async def test_user_group_uniqueness(make_engine):
sync_engine = make_engine(False)
metadata.drop_all(sync_engine)
metadata.create_all(sync_engine)

async with engine.acquire() as conn:
rory_group = await _create_group(conn, name="Rory Storm and the Hurricanes")
ringo = await _create_user(conn, "Ringo", rory_group)
Expand All @@ -50,6 +54,17 @@ async def test_user_group_uniqueness(make_engine):
user_to_groups.insert().values(uid=ringo.id, gid=rory_group.gid)
)

# Checks implementation of simcore_service_webserver/groups_api.py:get_group_from_gid
res: ResultProxy = await conn.execute(
groups.select().where(groups.c.gid == rory_group.gid)
)

the_one: Optional[RowProxy] = await res.first()
assert the_one.type == the_one["type"]

with pytest.raises(aiopg.sa.exc.ResourceClosedError):
await res.fetchone()


async def test_all_group(make_engine):
engine = await make_engine()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
""" Extends assertions for testing

"""
from aiohttp import web

from pprint import pformat
from typing import Type

from aiohttp import ClientResponse
from aiohttp.web import HTTPError, HTTPException, HTTPInternalServerError, HTTPNoContent
from servicelib.rest_responses import unwrap_envelope


async def assert_status(
response: web.Response, expected_cls: web.HTTPException, expected_msg: str = None
response: ClientResponse,
expected_cls: Type[HTTPException],
expected_msg: str = None,
):
"""
Asserts for enveloped responses
Expand All @@ -18,10 +22,10 @@ async def assert_status(
response.status == expected_cls.status_code
), f"received: ({data},{error}), \nexpected ({expected_cls.status_code}, {expected_msg})"

if issubclass(expected_cls, web.HTTPError):
if issubclass(expected_cls, HTTPError):
do_assert_error(data, error, expected_cls, expected_msg)

elif issubclass(expected_cls, web.HTTPNoContent):
elif issubclass(expected_cls, HTTPNoContent):
assert not data, pformat(data)
assert not error, pformat(error)
else:
Expand All @@ -37,14 +41,16 @@ async def assert_status(


async def assert_error(
response: web.Response, expected_cls: web.HTTPException, expected_msg: str = None
response: ClientResponse,
expected_cls: Type[HTTPException],
expected_msg: str = None,
):
data, error = unwrap_envelope(await response.json())
return do_assert_error(data, error, expected_cls, expected_msg)


def do_assert_error(
data, error, expected_cls: web.HTTPException, expected_msg: str = None
data, error, expected_cls: Type[HTTPException], expected_msg: str = None
):
assert not data, pformat(data)
assert error, pformat(error)
Expand All @@ -55,7 +61,7 @@ def do_assert_error(
if expected_msg:
assert expected_msg in err["message"]

if expected_cls != web.HTTPInternalServerError:
if expected_cls != HTTPInternalServerError:
# otherwise, code is exactly the name of the Exception class
assert expected_cls.__name__ == err["code"]

Expand Down
2 changes: 1 addition & 1 deletion services/sidecar/src/simcore_service_sidecar/log_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ async def _monitor_log_file(
# try to read line
line = await file_pointer.readline()
if not line:
asyncio.sleep(1)
await asyncio.sleep(1)
continue
log_type, parsed_line = await parse_line(line)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
import sqlalchemy as sa
from aiohttp import web
from aiopg.sa import SAConnection
from aiopg.sa.result import RowProxy
from aiopg.sa.engine import Engine
from aiopg.sa.result import ResultProxy, RowProxy
from servicelib.application_keys import APP_DB_ENGINE_KEY
from sqlalchemy import and_, literal_column
from sqlalchemy.dialects.postgresql import insert
Expand Down Expand Up @@ -348,9 +349,11 @@ async def delete_user_in_group(
)


async def get_group_from_gid(app: web.Application, gid: int) -> Dict:
engine = app[APP_DB_ENGINE_KEY]
async def get_group_from_gid(app: web.Application, gid: int) -> Optional[RowProxy]:
engine: Engine = app[APP_DB_ENGINE_KEY]

async with engine.acquire() as conn:
group = await conn.execute(sa.select([groups]).where(groups.c.gid == gid))
return await group.fetchone()
res: ResultProxy = await conn.execute(
groups.select().where(groups.c.gid == gid)
)
return await res.first()
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,15 @@

import trafaret as T
from aiohttp.web import Application
from pydantic import BaseSettings, Field, PositiveInt

from models_library.settings.redis import RedisConfig
from pydantic import BaseSettings, Field, PositiveInt
from servicelib.application_keys import APP_CONFIG_KEY

CONFIG_SECTION_NAME = "resource_manager"
APP_CLIENT_REDIS_CLIENT_KEY = __name__ + ".resource_manager.redis_client"
APP_CLIENT_REDIS_LOCK_KEY = __name__ + ".resource_manager.redis_lock"
APP_CLIENT_SOCKET_REGISTRY_KEY = __name__ + ".resource_manager.registry"
APP_RESOURCE_MANAGER_TASKS_KEY = __name__ + ".resource_manager.tasks.key"
APP_GARBAGE_COLLECTOR_KEY = __name__ + ".resource_manager.garbage_collector_key"


# lock names and format strings
Expand Down
Loading