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

* Missing await 

* Global exceptions were silencing CancellationError

* Catches all db errors

* Fixes typing and imports

* Fixes group_id str type and disables delete user mock

* Fixes typing and closes result

* Adapted gid type

Author: pcrespov

packages/postgres-database/tests/test_groups.py

@@ -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:
@@ -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)
@@ -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()

packages/pytest-simcore/src/pytest_simcore/helpers/utils_assert.py

@@ -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
@@ -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:
@@ -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)
@@ -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"]
 

services/sidecar/src/simcore_service_sidecar/log_parser.py

@@ -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)
 

services/web/server/src/simcore_service_webserver/groups_api.py

@@ -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
@@ -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()

services/web/server/src/simcore_service_webserver/resource_manager/config.py

@@ -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