From d873259f51ab9ceda6356462ab5097605c0b064d Mon Sep 17 00:00:00 2001 From: Sylvain <35365065+sanderegg@users.noreply.github.com> Date: Tue, 6 Aug 2024 10:05:33 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8FMaintenance:=20mypy=20postgre?= =?UTF-8?q?s=20database=20package=20(#6140)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/ci-testing-deploy.yml | 1 - packages/postgres-database/requirements/_test.in | 2 ++ packages/postgres-database/requirements/_test.txt | 12 ++++++++++++ packages/postgres-database/setup.cfg | 5 +++++ .../src/simcore_postgres_database/cli.py | 5 ++++- .../src/simcore_postgres_database/migration/env.py | 4 ++-- .../009c81406676_add_project_running_state.py | 9 +++++---- .../10b293fdcd56_alters_product_login_settings.py | 5 +++-- .../a8f0bacbbaef_product_issue_cols_nullable.py | 9 +++++---- .../src/simcore_postgres_database/models/base.py | 6 +++++- .../src/simcore_postgres_database/utils_aiopg_orm.py | 7 ++++--- .../src/simcore_postgres_database/utils_payments.py | 4 ++-- .../src/simcore_postgres_database/utils_users.py | 4 ++-- 13 files changed, 51 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci-testing-deploy.yml b/.github/workflows/ci-testing-deploy.yml index 97ecb8b62e9..fe7d56efdfc 100644 --- a/.github/workflows/ci-testing-deploy.yml +++ b/.github/workflows/ci-testing-deploy.yml @@ -1379,7 +1379,6 @@ jobs: run: ./ci/github/unit-testing/postgres-database.bash install - name: typecheck run: ./ci/github/unit-testing/postgres-database.bash typecheck - continue-on-error: true - name: test if: always() run: ./ci/github/unit-testing/postgres-database.bash test diff --git a/packages/postgres-database/requirements/_test.in b/packages/postgres-database/requirements/_test.in index d5b1f4e6235..3249c9c02b2 100644 --- a/packages/postgres-database/requirements/_test.in +++ b/packages/postgres-database/requirements/_test.in @@ -20,3 +20,5 @@ pytest-instafail pytest-runner pyyaml sqlalchemy[mypy] # adds Mypy / Pep-484 Support for ORM Mappings SEE https://docs.sqlalchemy.org/en/20/orm/extensions/mypy.html +types-docker +types-psycopg2 diff --git a/packages/postgres-database/requirements/_test.txt b/packages/postgres-database/requirements/_test.txt index 0feb92b7a9d..6f8682ebbf6 100644 --- a/packages/postgres-database/requirements/_test.txt +++ b/packages/postgres-database/requirements/_test.txt @@ -75,9 +75,21 @@ tomli==2.0.1 # coverage # mypy # pytest +types-docker==7.1.0.20240806 + # via -r requirements/_test.in +types-psycopg2==2.9.21.20240417 + # via -r requirements/_test.in +types-requests==2.32.0.20240712 + # via types-docker typing-extensions==4.12.2 # via # -c requirements/_base.txt # -c requirements/_migration.txt # mypy # sqlalchemy2-stubs +urllib3==2.2.2 + # via + # -c requirements/../../../requirements/constraints.txt + # -c requirements/_migration.txt + # types-docker + # types-requests diff --git a/packages/postgres-database/setup.cfg b/packages/postgres-database/setup.cfg index 3ef6a2ba84b..3d2590eba91 100644 --- a/packages/postgres-database/setup.cfg +++ b/packages/postgres-database/setup.cfg @@ -13,3 +13,8 @@ addopts = -W error::sqlalchemy.exc.SAWarning markers = acceptance_test: "marks tests as 'acceptance tests' i.e. does the system do what the user expects? Typically those are workflows." testit: "marks test to run during development" + +[mypy] +plugins = + pydantic.mypy + sqlalchemy.ext.mypy.plugin diff --git a/packages/postgres-database/src/simcore_postgres_database/cli.py b/packages/postgres-database/src/simcore_postgres_database/cli.py index b52e8146baa..5fa3cf22025 100644 --- a/packages/postgres-database/src/simcore_postgres_database/cli.py +++ b/packages/postgres-database/src/simcore_postgres_database/cli.py @@ -1,6 +1,7 @@ """ command line interface for migration """ + # pylint: disable=wildcard-import # pylint: disable=unused-wildcard-import @@ -159,14 +160,16 @@ def clean(): @main.command() def upgrade_and_close(): """Used in migration service program to discover, upgrade and close""" - + assert discover.callback # nosec for attempt in Retrying(wait=wait_fixed(5), after=after_log(log, logging.ERROR)): with attempt: if not discover.callback(): raise PostgresNotFoundError try: + assert info.callback # nosec info.callback() + assert upgrade.callback # nosec upgrade.callback(revision="head") info.callback() except Exception: # pylint: disable=broad-except diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/env.py b/packages/postgres-database/src/simcore_postgres_database/migration/env.py index b4ed815e55a..febfee09bbc 100644 --- a/packages/postgres-database/src/simcore_postgres_database/migration/env.py +++ b/packages/postgres-database/src/simcore_postgres_database/migration/env.py @@ -1,9 +1,8 @@ from logging.config import fileConfig from alembic import context -from sqlalchemy import engine_from_config, pool - from simcore_postgres_database.settings import target_metadatas +from sqlalchemy import engine_from_config, pool # this is the Alembic Config object, which provides # access to the values within the .ini file in use. @@ -15,6 +14,7 @@ if __name__ == "__main__": # swallows up all log messages from tests # only enable it during cli invocation + assert config.config_file_name is not None # nosec fileConfig(config.config_file_name) # add your model's MetaData object here diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/009c81406676_add_project_running_state.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/009c81406676_add_project_running_state.py index b3437caefbb..d709ba22924 100644 --- a/packages/postgres-database/src/simcore_postgres_database/migration/versions/009c81406676_add_project_running_state.py +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/009c81406676_add_project_running_state.py @@ -5,6 +5,7 @@ Create Date: 2020-10-12 08:38:40.807576+00:00 """ + import sqlalchemy as sa from alembic import op from sqlalchemy.dialects import postgresql @@ -87,7 +88,7 @@ def upgrade(): FAILED: "FAILED", } [ - op.execute( + op.execute( # type: ignore[func-returns-value] # no reason to change now sa.DDL( f""" UPDATE comp_pipeline @@ -99,7 +100,7 @@ def upgrade(): for old, new in migration_map.items() ] [ - op.execute( + op.execute( # type: ignore[func-returns-value] # no reason to change now sa.DDL( f""" UPDATE comp_tasks @@ -146,7 +147,7 @@ def downgrade(): "FAILED": FAILED, } [ - op.execute( + op.execute( # type: ignore[func-returns-value] # no reason to change now sa.DDL( f""" UPDATE comp_pipeline @@ -158,7 +159,7 @@ def downgrade(): for old, new in migration_map.items() ] [ - op.execute( + op.execute( # type: ignore[func-returns-value] # no reason to change now sa.DDL( f""" UPDATE comp_tasks diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/10b293fdcd56_alters_product_login_settings.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/10b293fdcd56_alters_product_login_settings.py index 328200f515d..a429d377742 100644 --- a/packages/postgres-database/src/simcore_postgres_database/migration/versions/10b293fdcd56_alters_product_login_settings.py +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/10b293fdcd56_alters_product_login_settings.py @@ -5,6 +5,7 @@ Create Date: 2023-01-14 21:12:56.182870+00:00 """ + import json import sqlalchemy as sa @@ -40,7 +41,7 @@ def upgrade(): "products", "login_settings", server_default=sa.text("'{}'::jsonb"), - existing_server_default=sa.text("'{\"two_factor_enabled\": false}'::jsonb"), + existing_server_default=sa.text("'{\"two_factor_enabled\": false}'::jsonb"), # type: ignore[arg-type] ) @@ -67,6 +68,6 @@ def downgrade(): "products", "login_settings", existing_type=postgresql.JSONB(astext_type=sa.Text()), - existing_server_default=sa.text("'{}'::jsonb"), + existing_server_default=sa.text("'{}'::jsonb"), # type: ignore[arg-type] server_default=sa.text("'{\"two_factor_enabled\": false}'::jsonb"), ) diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/a8f0bacbbaef_product_issue_cols_nullable.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/a8f0bacbbaef_product_issue_cols_nullable.py index ee73e5fea4d..d8c6f9e747b 100644 --- a/packages/postgres-database/src/simcore_postgres_database/migration/versions/a8f0bacbbaef_product_issue_cols_nullable.py +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/a8f0bacbbaef_product_issue_cols_nullable.py @@ -5,6 +5,7 @@ Create Date: 2022-08-24 13:33:30.104287+00:00 """ + import sqlalchemy as sa from alembic import op @@ -22,7 +23,7 @@ def upgrade(): "issues_login_url", existing_type=sa.VARCHAR(), nullable=True, - existing_server_default=sa.text( + existing_server_default=sa.text( # type: ignore[arg-type] "'https://github.com/ITISFoundation/osparc-simcore/issues'::character varying" ), ) @@ -31,7 +32,7 @@ def upgrade(): "issues_new_url", existing_type=sa.VARCHAR(), nullable=True, - existing_server_default=sa.text( + existing_server_default=sa.text( # type: ignore[arg-type] "'https://github.com/ITISFoundation/osparc-simcore/issues/new'::character varying" ), ) @@ -45,7 +46,7 @@ def downgrade(): "issues_new_url", existing_type=sa.VARCHAR(), nullable=False, - existing_server_default=sa.text( + existing_server_default=sa.text( # type: ignore[arg-type] "'https://github.com/ITISFoundation/osparc-simcore/issues/new'::character varying" ), ) @@ -54,7 +55,7 @@ def downgrade(): "issues_login_url", existing_type=sa.VARCHAR(), nullable=False, - existing_server_default=sa.text( + existing_server_default=sa.text( # type: ignore[arg-type] "'https://github.com/ITISFoundation/osparc-simcore/issues'::character varying" ), ) diff --git a/packages/postgres-database/src/simcore_postgres_database/models/base.py b/packages/postgres-database/src/simcore_postgres_database/models/base.py index bf66f1731e1..cf4e28001bc 100644 --- a/packages/postgres-database/src/simcore_postgres_database/models/base.py +++ b/packages/postgres-database/src/simcore_postgres_database/models/base.py @@ -3,10 +3,14 @@ - Collects all table's schemas - Metadata object needed to explicitly define table schemas """ + +from typing import cast + import sqlalchemy.orm +from sqlalchemy.ext.declarative import DeclarativeMeta # DO NOT inheriting from _base. Use instead explicit table definitions # See https://docs.sqlalchemy.org/en/latest/orm/mapping_styles.html#classical-mappings -Base = sqlalchemy.orm.declarative_base() +Base = cast(DeclarativeMeta, sqlalchemy.orm.declarative_base()) metadata = Base.metadata diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_aiopg_orm.py b/packages/postgres-database/src/simcore_postgres_database/utils_aiopg_orm.py index f803eb036f0..4a582769d3e 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_aiopg_orm.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_aiopg_orm.py @@ -6,11 +6,12 @@ - the new async sqlalchemy ORM https://docs.sqlalchemy.org/en/14/orm/ - https://piccolo-orm.readthedocs.io/en/latest/index.html """ + # pylint: disable=no-value-for-parameter import functools import operator -from typing import Any, Generic, TypeVar +from typing import Any, Generic, TypeVar, cast import sqlalchemy as sa from aiopg.sa.connection import SAConnection @@ -246,7 +247,7 @@ async def update( query, is_scalar = self._append_returning(returning_cols, query) if is_scalar: - return await self._conn.scalar(query) + return cast(RowUId, await self._conn.scalar(query)) result: ResultProxy = await self._conn.execute(query) row: RowProxy | None = await result.first() @@ -261,7 +262,7 @@ async def insert( query, is_scalar = self._append_returning(returning_cols, query) if is_scalar: - return await self._conn.scalar(query) + return cast(RowUId, await self._conn.scalar(query)) result: ResultProxy = await self._conn.execute(query) row: RowProxy | None = await result.first() diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_payments.py b/packages/postgres-database/src/simcore_postgres_database/utils_payments.py index 68fd1a35eb3..7202eb21d74 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_payments.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_payments.py @@ -88,7 +88,7 @@ async def update_payment_transaction_state( msg = f"cannot update state with {completion_state=} since it is already initiated" raise ValueError(msg) - optional = {} + optional: dict[str, str | None] = {} if state_message: optional["state_message"] = state_message @@ -132,7 +132,7 @@ async def update_payment_transaction_state( ) row = await result.first() assert row, "execute above should have caught this" # nosec - + assert isinstance(row, RowProxy) # nosec return row diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_users.py b/packages/postgres-database/src/simcore_postgres_database/utils_users.py index d2f97446b97..806d950fee5 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_users.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_users.py @@ -2,7 +2,6 @@ i.e. models.users main table and all its relations """ - import re import secrets import string @@ -152,7 +151,8 @@ async def get_billing_details(conn: SAConnection, user_id: int) -> RowProxy | No ) .where(users.c.id == user_id) ) - return await result.fetchone() + value: RowProxy | None = await result.fetchone() + return value @staticmethod async def get_role(conn: SAConnection, user_id: int) -> UserRole: