Merge branch 'master' into fix/debt-msg

Author: matusdrobuliak66

.github/mergify.yml

@@ -0,0 +1,46 @@
+pull_request_rules:
+    - name: Automatic merge
+      description: Conditions required for automatic merging of a PR
+      conditions:
+        # let Mergify know that the PR can be merged (added manually)
+        - label = 🤖-automerge
+        # block Mergify from merging the PR (added manually)
+        - label != 🤖-do-not-merge
+
+        # list of CI checks that need to pass in order for the PR to be merged
+        - check-success=unit-tests
+        - check-success=integration-tests
+        - check-success=system-tests
+
+        # Check for required reviews
+        - "#approved-reviews-by>=2"  # Requires 2 approving reviews
+        - "#changes-requested-reviews-by=0"  # No changes requested
+        - "#review-threads-unresolved=0"  # All review threads resolved
+
+        # Optional but recommended checks
+        - base=master
+        - -draft  # PR is not in draft state
+        - -conflict  # No merge conflicts
+      actions:
+        merge:
+
+    - name: automatic update for PR ready to be merged
+      conditions:
+        - -draft
+        - label=🤖-automerge
+      actions:
+        update:
+
+    - name: retry CI on failure
+      conditions:
+        - base=master
+        - or:
+          - check-failure=unit-tests
+          - check-failure=integration-tests
+          - check-failure=system-tests
+      actions:
+        github_actions:
+          workflow:
+            dispatch:
+            - workflow: .github/workflows/ci-testing-deploy.yml
+

.ruff.toml

@@ -1,5 +1,5 @@
 
-select = [
+lint.select = [
   "A",     # [https://pypi.org/project/flake8-builtins/]
   "ARG",   # [https://pypi.org/project/flake8-unused-arguments/]
   "ASYNC", # [https://pypi.org/project/flake8-async/]
@@ -40,7 +40,7 @@ select = [
   "W",     # [https://pypi.org/project/pycodestyle/] warnings
   "YTT",   # [https://pypi.org/project/flake8-2020/]
 ]
-ignore = [
+lint.ignore = [
   "E501",   # line too long, handled by black
   "S101",   # use of `assert` detected hanbled by pylance, does not support noseq
   "TID252", # [*] Relative imports from parent modules are banned
@@ -50,7 +50,7 @@ ignore = [
 target-version = "py311"
 
 
-[per-file-ignores]
+[lint.per-file-ignores]
 "**/{tests,pytest_simcore}/**" = [
   "T201",    # print found
   "ARG001",  # unused function argument
@@ -64,10 +64,10 @@ target-version = "py311"
   "FBT001",  # Boolean positional arg in function definition
 ]
 
-[flake8-pytest-style]
+[lint.flake8-pytest-style]
 fixture-parentheses = false
 parametrize-names-type = "csv"
 
 
-[pylint]
+[lint.pylint]
 max-args = 10

packages/postgres-database/tests/conftest.py

@@ -5,19 +5,21 @@
 
 import uuid
 import warnings
-from collections.abc import AsyncIterator, Awaitable, Callable, Iterator
+from collections.abc import AsyncIterator, Awaitable, Callable, Iterable, Iterator
 from pathlib import Path
 
 import aiopg.sa
 import aiopg.sa.exc
 import pytest
 import simcore_postgres_database.cli
 import sqlalchemy as sa
+import sqlalchemy.engine
 import yaml
 from aiopg.sa.connection import SAConnection
 from aiopg.sa.engine import Engine
 from aiopg.sa.result import ResultProxy, RowProxy
 from faker import Faker
+from pytest_simcore.helpers import postgres_tools
 from pytest_simcore.helpers.faker_factories import (
     random_group,
     random_project,
@@ -71,20 +73,15 @@ def postgres_service(docker_services, docker_ip, docker_compose_file) -> str:
     return dsn
 
 
-@pytest.fixture
-def make_engine(
-    postgres_service: str,
-) -> Callable[[bool], Awaitable[Engine] | sa.engine.base.Engine]:
-    dsn = postgres_service
-
-    def _make(is_async=True) -> Awaitable[Engine] | sa.engine.base.Engine:
-        return aiopg.sa.create_engine(dsn) if is_async else sa.create_engine(dsn)
-
-    return _make
+@pytest.fixture(scope="session")
+def sync_engine(postgres_service: str) -> Iterable[sqlalchemy.engine.Engine]:
+    _engine: sqlalchemy.engine.Engine = sa.create_engine(url=postgres_service)
+    yield _engine
+    _engine.dispose()
 
 
 @pytest.fixture
-def make_asyncpg_engine(postgres_service: str) -> Callable[[bool], AsyncEngine]:
+def _make_asyncpg_engine(postgres_service: str) -> Callable[[bool], AsyncEngine]:
     # NOTE: users is responsible of `await engine.dispose()`
     dsn = postgres_service.replace("postgresql://", "postgresql+asyncpg://")
     minsize = 1
@@ -127,10 +124,10 @@ def db_metadata() -> sa.MetaData:
 
 @pytest.fixture(params=["sqlModels", "alembicMigration"])
 def pg_sa_engine(
-    make_engine: Callable,
+    sync_engine: sqlalchemy.engine.Engine,
     db_metadata: sa.MetaData,
     request: pytest.FixtureRequest,
-) -> Iterator[sa.engine.Engine]:
+) -> Iterator[sqlalchemy.engine.Engine]:
     """
     Runs migration to create tables and return a sqlalchemy engine
 
@@ -144,7 +141,6 @@ def pg_sa_engine(
     # the tables, i.e. when no migration mechanism are in place
     # Best is therefore to start from scratch and delete all at
     # the end
-    sync_engine = make_engine(is_async=False)
 
     # NOTE: ALL is deleted before
     db_metadata.drop_all(sync_engine)
@@ -165,22 +161,20 @@ def pg_sa_engine(
 
     yield sync_engine
 
-    # NOTE: ALL is deleted after
-    with sync_engine.begin() as conn:
-        conn.execute(sa.DDL("DROP TABLE IF EXISTS alembic_version"))
-    db_metadata.drop_all(sync_engine)
-    sync_engine.dispose()
+    postgres_tools.force_drop_all_tables(sync_engine)
 
 
 @pytest.fixture
 async def aiopg_engine(
-    pg_sa_engine: sa.engine.Engine, make_engine: Callable
+    pg_sa_engine: sqlalchemy.engine.Engine,
+    postgres_service: str,
 ) -> AsyncIterator[Engine]:
     """
     Return an aiopg.sa engine connected to a responsive and migrated pg database
     """
-
-    aiopg_sa_engine = await make_engine(is_async=True)
+    # first start sync
+    assert pg_sa_engine.url.database
+    assert postgres_service.endswith(pg_sa_engine.url.database)
 
     warnings.warn(
         "The 'aiopg_engine' is deprecated since we are replacing `aiopg` library by `sqlalchemy.ext.asyncio`."
@@ -190,12 +184,8 @@ async def aiopg_engine(
         stacklevel=2,
     )
 
-    yield aiopg_sa_engine
-
-    # closes async-engine connections and terminates
-    aiopg_sa_engine.close()
-    await aiopg_sa_engine.wait_closed()
-    aiopg_sa_engine.terminate()
+    async with aiopg.sa.create_engine(dsn=postgres_service) as aiopg_sa_engine:
+        yield aiopg_sa_engine
 
 
 @pytest.fixture
@@ -208,15 +198,15 @@ async def connection(aiopg_engine: Engine) -> AsyncIterator[SAConnection]:
 @pytest.fixture
 async def asyncpg_engine(  # <-- WE SHOULD USE THIS ONE
     is_pdb_enabled: bool,
-    pg_sa_engine: sa.engine.Engine,
-    make_asyncpg_engine: Callable[[bool], AsyncEngine],
+    pg_sa_engine: sqlalchemy.engine.Engine,
+    _make_asyncpg_engine: Callable[[bool], AsyncEngine],
 ) -> AsyncIterator[AsyncEngine]:
 
     assert (
         pg_sa_engine
     ), "Ensures pg db up, responsive, init (w/ tables) and/or migrated"
 
-    _apg_engine = make_asyncpg_engine(is_pdb_enabled)
+    _apg_engine = _make_asyncpg_engine(is_pdb_enabled)
 
     yield _apg_engine
 
@@ -229,9 +219,7 @@ async def asyncpg_engine(  # <-- WE SHOULD USE THIS ONE
 
 
 @pytest.fixture
-def create_fake_group(
-    make_engine: Callable[..., Awaitable[Engine] | sa.engine.base.Engine]
-) -> Iterator[Callable]:
+def create_fake_group(sync_engine: sqlalchemy.engine.Engine) -> Iterator[Callable]:
     """factory to create standard group"""
     created_ids = []
 
@@ -250,16 +238,13 @@ async def _creator(conn: SAConnection, **overrides) -> RowProxy:
 
     yield _creator
 
-    sync_engine = make_engine(is_async=False)
-    assert isinstance(sync_engine, sa.engine.Engine)
+    assert isinstance(sync_engine, sqlalchemy.engine.Engine)
     with sync_engine.begin() as conn:
         conn.execute(sa.delete(groups).where(groups.c.gid.in_(created_ids)))
 
 
 @pytest.fixture
-def create_fake_user(
-    make_engine: Callable[..., Awaitable[Engine] | sa.engine.base.Engine]
-) -> Iterator[Callable]:
+def create_fake_user(sync_engine: sqlalchemy.engine.Engine) -> Iterator[Callable]:
     """factory to create a user w/ or w/o a standard group"""
 
     created_ids = []
@@ -290,8 +275,7 @@ async def _creator(conn, group: RowProxy | None = None, **overrides) -> RowProxy
 
     yield _creator
 
-    sync_engine = make_engine(is_async=False)
-    assert isinstance(sync_engine, sa.engine.Engine)
+    assert isinstance(sync_engine, sqlalchemy.engine.Engine)
     with sync_engine.begin() as conn:
         conn.execute(users.delete().where(users.c.id.in_(created_ids)))
 

packages/postgres-database/tests/test_models_groups.py

@@ -3,18 +3,14 @@
 # pylint: disable=unused-variable
 # pylint: disable=too-many-arguments
 
-
-from collections.abc import AsyncIterator, Awaitable, Callable
+from collections.abc import Callable
 
 import aiopg.sa.exc
 import pytest
-import sqlalchemy as sa
 from aiopg.sa.connection import SAConnection
-from aiopg.sa.engine import Engine
 from aiopg.sa.result import ResultProxy, RowProxy
 from psycopg2.errors import ForeignKeyViolation, RaiseException, UniqueViolation
 from pytest_simcore.helpers.faker_factories import random_user
-from simcore_postgres_database.models.base import metadata
 from simcore_postgres_database.webserver_models import (
     GroupType,
     groups,
@@ -24,21 +20,6 @@
 from sqlalchemy import func, literal_column, select
 
 
-@pytest.fixture
-async def connection(
-    make_engine: Callable[[bool], Awaitable[Engine] | sa.engine.base.Engine]
-) -> AsyncIterator[SAConnection]:
-    engine = await make_engine()
-    sync_engine = make_engine(is_async=False)
-    metadata.drop_all(sync_engine)
-    metadata.create_all(sync_engine)
-
-    async with engine.acquire() as conn:
-        yield conn
-
-    metadata.drop_all(sync_engine)
-
-
 async def test_user_group_uniqueness(
     connection: SAConnection,
     create_fake_group: Callable,

packages/postgres-database/tests/test_uniqueness_in_comp_tasks.py

@@ -1,13 +1,18 @@
-# pylint:disable=no-value-for-parameter
-# pylint:disable=unused-variable
-# pylint:disable=unused-argument
-# pylint:disable=redefined-outer-name
+# pylint: disable=redefined-outer-name
+# pylint: disable=unused-argument
+# pylint: disable=unused-variable
+# pylint: disable=too-many-arguments
 
 import json
+from collections.abc import AsyncIterator
 
+import aiopg.sa.engine
+import aiopg.sa.exc
 import pytest
 import sqlalchemy as sa
+import sqlalchemy.engine
 from psycopg2.errors import UniqueViolation  # pylint: disable=no-name-in-module
+from pytest_simcore.helpers import postgres_tools
 from pytest_simcore.helpers.faker_factories import fake_pipeline, fake_task_factory
 from simcore_postgres_database.models.base import metadata
 from simcore_postgres_database.webserver_models import comp_pipeline, comp_tasks
@@ -16,24 +21,25 @@
 
 
 @pytest.fixture
-async def engine(make_engine):
-    engine = await make_engine()
-    sync_engine = make_engine(is_async=False)
+async def engine(
+    sync_engine: sqlalchemy.engine.Engine,
+    aiopg_engine: aiopg.sa.engine.Engine,
+) -> AsyncIterator[aiopg.sa.engine.Engine]:
+
     metadata.drop_all(sync_engine)
     metadata.create_all(sync_engine)
 
-    async with engine.acquire() as conn:
+    async with aiopg_engine.acquire() as conn:
         await conn.execute(
             comp_pipeline.insert().values(**fake_pipeline(project_id="PA"))
         )
         await conn.execute(
             comp_pipeline.insert().values(**fake_pipeline(project_id="PB"))
         )
 
-    yield engine
+    yield aiopg_engine
 
-    engine.close()
-    await engine.wait_closed()
+    postgres_tools.force_drop_all_tables(sync_engine)
 
 
 async def test_unique_project_node_pairs(engine):

packages/postgres-database/tests/test_utils_migration.py

@@ -3,10 +3,10 @@
 # pylint: disable=unused-argument
 # pylint: disable=unused-variable
 
-from collections.abc import Callable
 
 import pytest
 import simcore_postgres_database.cli
+import sqlalchemy.engine
 from alembic.script.revision import MultipleHeads
 from simcore_postgres_database.utils_migration import get_current_head
 from sqlalchemy import inspect
@@ -23,8 +23,8 @@ def test_migration_has_no_branches():
         )
 
 
-def test_migration_upgrade_downgrade(make_engine: Callable):
-    sync_engine = make_engine(is_async=False)
+def test_migration_upgrade_downgrade(sync_engine: sqlalchemy.engine.Engine):
+
     assert sync_engine
     assert simcore_postgres_database.cli.discover.callback
     assert simcore_postgres_database.cli.upgrade.callback