From d543b9353c07da1896943e8932b5f131d5c12df7 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 17:43:04 +0100 Subject: [PATCH 01/21] cleanup models-library --- .../src/models_library/docker.py | 12 ++------- .../src/models_library/projects_nodes.py | 9 +++---- .../src/models_library/services.py | 12 +++++++-- .../src/models_library/services_resources.py | 25 ++++++++++++------- .../tests/test_service_resources.py | 13 +++++----- 5 files changed, 37 insertions(+), 34 deletions(-) diff --git a/packages/models-library/src/models_library/docker.py b/packages/models-library/src/models_library/docker.py index 22cfce217a8..0c7f3f6e784 100644 --- a/packages/models-library/src/models_library/docker.py +++ b/packages/models-library/src/models_library/docker.py @@ -5,17 +5,9 @@ from models_library.projects import ProjectID from models_library.projects_nodes import NodeID from models_library.users import UserID -from pydantic import BaseModel, ConstrainedStr, Field, constr +from pydantic import BaseModel, ConstrainedStr, Field -from .basic_regex import ( - DOCKER_GENERIC_TAG_KEY_RE, - DOCKER_IMAGE_KEY_RE, - DOCKER_IMAGE_VERSION_RE, - DOCKER_LABEL_KEY_REGEX, -) - -DockerImageKey = constr(regex=DOCKER_IMAGE_KEY_RE) -DockerImageVersion = constr(regex=DOCKER_IMAGE_VERSION_RE) +from .basic_regex import DOCKER_GENERIC_TAG_KEY_RE, DOCKER_LABEL_KEY_REGEX class DockerLabelKey(ConstrainedStr): diff --git a/packages/models-library/src/models_library/projects_nodes.py b/packages/models-library/src/models_library/projects_nodes.py index f661319d66c..64bb52ce0e8 100644 --- a/packages/models-library/src/models_library/projects_nodes.py +++ b/packages/models-library/src/models_library/projects_nodes.py @@ -18,7 +18,6 @@ validator, ) -from .basic_regex import VERSION_RE from .basic_types import EnvVarKey from .projects_access import AccessEnum from .projects_nodes_io import ( @@ -30,7 +29,7 @@ ) from .projects_nodes_ui import Position from .projects_state import RunningState -from .services import PROPERTY_KEY_RE, SERVICE_KEY_RE +from .services import PROPERTY_KEY_RE, ServiceKey, ServiceVersion # NOTE: WARNING the order here matters @@ -100,20 +99,18 @@ class Config: class Node(BaseModel): - key: str = Field( + key: ServiceKey = Field( ..., description="distinctive name for the node based on the docker registry path", - regex=SERVICE_KEY_RE, examples=[ "simcore/services/comp/itis/sleeper", "simcore/services/dynamic/3dviewer", "simcore/services/frontend/file-picker", ], ) - version: str = Field( + version: ServiceVersion = Field( ..., description="semantic version number of the node", - regex=VERSION_RE, examples=["1.0.0", "0.0.1"], ) label: str = Field( diff --git a/packages/models-library/src/models_library/services.py b/packages/models-library/src/models_library/services.py index f7bf16f2d14..7e04f15b717 100644 --- a/packages/models-library/src/models_library/services.py +++ b/packages/models-library/src/models_library/services.py @@ -4,6 +4,7 @@ python -c "from models_library.services import ServiceDockerData as cls; print(cls.schema_json(indent=2))" > services-schema.json """ +import re from datetime import datetime from enum import Enum from typing import Any, Optional, Union @@ -11,6 +12,7 @@ from pydantic import ( BaseModel, + ConstrainedStr, EmailStr, Extra, Field, @@ -54,8 +56,14 @@ ServicePortKey = constr(regex=PROPERTY_KEY_RE) FileName = constr(regex=FILENAME_RE) -ServiceKey = constr(regex=KEY_RE) -ServiceVersion = constr(regex=VERSION_RE) + +class ServiceKey(ConstrainedStr): + regex = re.compile(SERVICE_KEY_RE) + + +class ServiceVersion(ConstrainedStr): + regex = re.compile(VERSION_RE) + RunID = UUID diff --git a/packages/models-library/src/models_library/services_resources.py b/packages/models-library/src/models_library/services_resources.py index bebf6bdd200..8722d158b7e 100644 --- a/packages/models-library/src/models_library/services_resources.py +++ b/packages/models-library/src/models_library/services_resources.py @@ -1,13 +1,13 @@ import logging from typing import Any, Final, Union +from models_library.docker import DockerGenericTag from pydantic import ( BaseModel, ByteSize, Field, StrictFloat, StrictInt, - constr, parse_obj_as, root_validator, ) @@ -16,13 +16,14 @@ logger = logging.getLogger(__name__) -DockerImage = constr(regex=r"[\w/-]+:[\w.@]+") -DockerComposeServiceName = constr(regex=r"^[a-zA-Z0-9._-]+$") + ResourceName = str # NOTE: replace hard coded `container` with function which can # extract the name from the `service_key` or `registry_address/service_key` -DEFAULT_SINGLE_SERVICE_NAME: Final[DockerComposeServiceName] = "container" +DEFAULT_SINGLE_SERVICE_NAME: Final[DockerGenericTag] = parse_obj_as( + DockerGenericTag, "container" +) MEMORY_50MB: Final[int] = parse_obj_as(ByteSize, "50mib") MEMORY_250MB: Final[int] = parse_obj_as(ByteSize, "250mib") @@ -58,7 +59,7 @@ class Config: class ImageResources(BaseModel): - image: DockerImage = Field( + image: DockerGenericTag = Field( ..., description=( "Used by the frontend to provide a context for the users." @@ -87,23 +88,29 @@ class Config: } -ServiceResourcesDict = dict[DockerComposeServiceName, ImageResources] +ServiceResourcesDict = dict[DockerGenericTag, ImageResources] class ServiceResourcesDictHelpers: @staticmethod def create_from_single_service( - image: DockerComposeServiceName, resources: ResourcesDict + image: DockerGenericTag, + resources: ResourcesDict, ) -> ServiceResourcesDict: return parse_obj_as( ServiceResourcesDict, - {DEFAULT_SINGLE_SERVICE_NAME: {"image": image, "resources": resources}}, + { + DEFAULT_SINGLE_SERVICE_NAME: { + "image": image, + "resources": resources, + } + }, ) @staticmethod def create_jsonable( service_resources: ServiceResourcesDict, - ) -> dict[DockerComposeServiceName, Any]: + ) -> dict[DockerGenericTag, Any]: return jsonable_encoder(service_resources) class Config: diff --git a/packages/models-library/tests/test_service_resources.py b/packages/models-library/tests/test_service_resources.py index 56f1409c054..25d00df884b 100644 --- a/packages/models-library/tests/test_service_resources.py +++ b/packages/models-library/tests/test_service_resources.py @@ -5,9 +5,8 @@ from typing import Any import pytest +from models_library.docker import DockerGenericTag from models_library.services_resources import ( - DockerComposeServiceName, - DockerImage, ImageResources, ResourcesDict, ResourceValue, @@ -28,7 +27,7 @@ ), ) def test_compose_image(example: str) -> None: - parse_obj_as(DockerImage, example) + parse_obj_as(DockerGenericTag, example) @pytest.fixture @@ -39,8 +38,8 @@ def resources_dict() -> ResourcesDict: @pytest.fixture -def compose_image() -> DockerImage: - return parse_obj_as(DockerImage, "image:latest") +def compose_image() -> DockerGenericTag: + return parse_obj_as(DockerGenericTag, "image:latest") def _ensure_resource_value_is_an_object(data: ResourcesDict) -> None: @@ -74,7 +73,7 @@ def test_image_resources_parsed_as_expected() -> None: "example", ServiceResourcesDictHelpers.Config.schema_extra["examples"] ) def test_service_resource_parsed_as_expected( - example: dict[DockerComposeServiceName, Any], compose_image: DockerImage + example: dict[DockerGenericTag, Any], compose_image: DockerGenericTag ) -> None: def _assert_service_resources_dict( service_resources_dict: ServiceResourcesDict, @@ -103,7 +102,7 @@ def _assert_service_resources_dict( @pytest.mark.parametrize( "example", ServiceResourcesDictHelpers.Config.schema_extra["examples"] ) -def test_create_jsonable_dict(example: dict[DockerComposeServiceName, Any]) -> None: +def test_create_jsonable_dict(example: dict[DockerGenericTag, Any]) -> None: service_resources_dict: ServiceResourcesDict = parse_obj_as( ServiceResourcesDict, example ) From bef868b0651dd9fd79d53a2f847d110d6a42c072 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 17:46:21 +0100 Subject: [PATCH 02/21] ensure API actually checks for correct service naming --- .../api/routes/services_resources.py | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/api/routes/services_resources.py b/services/catalog/src/simcore_service_catalog/api/routes/services_resources.py index b9967e210ad..5917d844834 100644 --- a/services/catalog/src/simcore_service_catalog/api/routes/services_resources.py +++ b/services/catalog/src/simcore_service_catalog/api/routes/services_resources.py @@ -4,13 +4,13 @@ from typing import Any, Final, Optional, cast import yaml -from aiocache import cached from fastapi import APIRouter, Depends, HTTPException, status -from models_library.docker import DockerImageKey, DockerImageVersion +from models_library.docker import DockerGenericTag from models_library.service_settings_labels import ( ComposeSpecLabel, SimcoreServiceSettingLabelEntry, ) +from models_library.services import ServiceKey, ServiceVersion from models_library.services_resources import ( ImageResources, ResourcesDict, @@ -23,7 +23,6 @@ from ...db.repositories.services import ServicesRepository from ...models.domain.group import GroupAtDB from ...models.schemas.constants import ( - DIRECTOR_CACHING_TTL, RESPONSE_MODEL_POLICY, SIMCORE_SERVICE_SETTINGS_LABELS, ) @@ -44,11 +43,11 @@ SIMCORE_SERVICE_COMPOSE_SPEC_LABEL: Final[str] = "simcore.service.compose-spec" -def _from_service_settings( +def _resources_from_settings( settings: list[SimcoreServiceSettingLabelEntry], default_service_resources: ResourcesDict, - service_key: DockerImageKey, - service_version: DockerImageVersion, + service_key: ServiceKey, + service_version: ServiceVersion, ) -> ResourcesDict: # filter resource entries resource_entries = filter(lambda entry: entry.name.lower() == "resources", settings) @@ -87,7 +86,7 @@ def _from_service_settings( async def _get_service_labels( - director_client: DirectorApi, key: DockerImageKey, version: DockerImageVersion + director_client: DirectorApi, key: ServiceKey, version: ServiceVersion ) -> Optional[dict[str, Any]]: try: service_labels = cast( @@ -128,19 +127,15 @@ def _get_service_settings( response_model=ServiceResourcesDict, **RESPONSE_MODEL_POLICY, ) -@cached( - ttl=DIRECTOR_CACHING_TTL, - key_builder=lambda f, *args, **kwargs: f"{f.__name__}_{kwargs.get('user_id', 'default')}_{kwargs['service_key']}_{kwargs['service_version']}", -) async def get_service_resources( - service_key: DockerImageKey, - service_version: DockerImageVersion, + service_key: ServiceKey, + service_version: ServiceVersion, director_client: DirectorApi = Depends(get_director_api), default_service_resources: ResourcesDict = Depends(get_default_service_resources), services_repo: ServicesRepository = Depends(get_repository(ServicesRepository)), user_groups: list[GroupAtDB] = Depends(list_user_groups), ) -> ServiceResourcesDict: - image_version = f"{service_key}:{service_version}" + image_version = parse_obj_as(DockerGenericTag, f"{service_key}:{service_version}") if is_function_service(service_key): return ServiceResourcesDictHelpers.create_from_single_service( image_version, default_service_resources @@ -164,7 +159,7 @@ async def get_service_resources( if service_spec is None: # no compose specifications -> single service service_settings = _get_service_settings(service_labels) - service_resources = _from_service_settings( + service_resources = _resources_from_settings( service_settings, default_service_resources, service_key, service_version ) user_specific_service_specs = await services_repo.get_service_specifications( @@ -207,7 +202,7 @@ async def get_service_resources( spec_service_resources: ResourcesDict = default_service_resources else: spec_service_settings = _get_service_settings(spec_service_labels) - spec_service_resources: ResourcesDict = _from_service_settings( + spec_service_resources: ResourcesDict = _resources_from_settings( spec_service_settings, default_service_resources, service_key, @@ -227,7 +222,10 @@ async def get_service_resources( ) service_to_resources[spec_key] = ImageResources.parse_obj( - {"image": image, "resources": spec_service_resources} + { + "image": image, + "resources": spec_service_resources, + } ) return service_to_resources From 0acc32be39d0b1bbc086f720b07d92f24ee00260 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 17:46:56 +0100 Subject: [PATCH 03/21] types --- .../simcore_service_catalog/services/director.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/services/director.py b/services/catalog/src/simcore_service_catalog/services/director.py index a2263a09d96..7c9a4cf4d02 100644 --- a/services/catalog/src/simcore_service_catalog/services/director.py +++ b/services/catalog/src/simcore_service_catalog/services/director.py @@ -1,7 +1,7 @@ import asyncio import functools import logging -from typing import Any, Awaitable, Callable, Dict, List, Optional, Union +from typing import Any, Awaitable, Callable, Optional, Union import httpx from fastapi import FastAPI, HTTPException @@ -66,7 +66,9 @@ async def close_director(app: FastAPI) -> None: # DIRECTOR API CLASS --------------------------------------------- -def safe_request(request_func: Callable[..., Awaitable[httpx.Response]]): +def safe_request( + request_func: Callable[..., Awaitable[httpx.Response]] +) -> Callable[..., Awaitable[Union[list[Any], dict[str, Any]]]]: """ Creates a context for safe inter-process communication (IPC) """ @@ -74,7 +76,7 @@ def safe_request(request_func: Callable[..., Awaitable[httpx.Response]]): def _unenvelope_or_raise_error( resp: httpx.Response, - ) -> Union[List[Any], Dict[str, Any]]: + ) -> Union[list[Any], dict[str, Any]]: """ Director responses are enveloped If successful response, we un-envelop it and return data as a dict @@ -105,7 +107,9 @@ def _unenvelope_or_raise_error( return data or {} @functools.wraps(request_func) - async def request_wrapper(zelf: "DirectorApi", path: str, *args, **kwargs): + async def request_wrapper( + zelf: "DirectorApi", path: str, *args, **kwargs + ) -> Union[list[Any], dict[str, Any]]: normalized_path = path.lstrip("/") try: resp = await request_func(zelf, path=normalized_path, *args, **kwargs) @@ -153,7 +157,7 @@ async def get(self, path: str) -> httpx.Response: return await self.client.get(path, timeout=20.0) @safe_request - async def put(self, path: str, body: Dict) -> httpx.Response: + async def put(self, path: str, body: dict) -> httpx.Response: return await self.client.put(path, json=body) async def is_responsive(self) -> bool: From 4008b27cc1b133700604248abc01c44b591ec2cd Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 18:24:32 +0100 Subject: [PATCH 04/21] mypy --- .../src/simcore_service_catalog/utils/service_resources.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/catalog/src/simcore_service_catalog/utils/service_resources.py b/services/catalog/src/simcore_service_catalog/utils/service_resources.py index fdbb1452c2a..1b6b7ddcbc9 100644 --- a/services/catalog/src/simcore_service_catalog/utils/service_resources.py +++ b/services/catalog/src/simcore_service_catalog/utils/service_resources.py @@ -10,7 +10,7 @@ def parse_generic_resource(generic_resources: list[Any]) -> ResourcesDict: - service_resources = {} + service_resources: ResourcesDict = {} for res in generic_resources: if not isinstance(res, dict): continue From e6fc9a05cfed225eec4c38d69012a7eface73056 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 18:24:59 +0100 Subject: [PATCH 05/21] mypy --- .../simcore_service_catalog/models/schemas/meta.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/models/schemas/meta.py b/services/catalog/src/simcore_service_catalog/models/schemas/meta.py index dd23eced796..0899035ad54 100644 --- a/services/catalog/src/simcore_service_catalog/models/schemas/meta.py +++ b/services/catalog/src/simcore_service_catalog/models/schemas/meta.py @@ -1,19 +1,23 @@ -from typing import Dict, Optional +import re +from typing import Optional -from pydantic import BaseModel, Field, constr +from pydantic import BaseModel, ConstrainedStr, Field # TODO: review this RE # use https://www.python.org/dev/peps/pep-0440/#version-scheme # or https://www.python.org/dev/peps/pep-0440/#appendix-b-parsing-version-strings-with-regular-expressions # VERSION_RE = r"^(0|[1-9]\d*)(\.(0|[1-9]\d*)){2}(-(0|[1-9]\d*|\d*[-a-zA-Z][-\da-zA-Z]*)(\.(0|[1-9]\d*|\d*[-a-zA-Z][-\da-zA-Z]*))*)?(\+[-\da-zA-Z]+(\.[-\da-zA-Z-]+)*)?$" -VersionStr = constr(regex=VERSION_RE) + + +class VersionStr(ConstrainedStr): + regex = re.compile(VERSION_RE) class Meta(BaseModel): name: str version: VersionStr - released: Optional[Dict[str, VersionStr]] = Field( + released: Optional[dict[str, VersionStr]] = Field( None, description="Maps every route's path tag with a released version" ) From 6fbb7c4ce2cb223cc9a1f297fff5e8596de6ad97 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 18:46:19 +0100 Subject: [PATCH 06/21] mypy --- .../api/dependencies/database.py | 6 ++-- .../api/routes/meta.py | 9 ++--- .../db/repositories/groups.py | 33 ++++++++++++------- .../db/repositories/services.py | 4 +-- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/api/dependencies/database.py b/services/catalog/src/simcore_service_catalog/api/dependencies/database.py index 2b0118662f3..d0de2107be5 100644 --- a/services/catalog/src/simcore_service_catalog/api/dependencies/database.py +++ b/services/catalog/src/simcore_service_catalog/api/dependencies/database.py @@ -1,5 +1,5 @@ import logging -from typing import AsyncGenerator, Callable, Type +from typing import AsyncGenerator, Callable from fastapi import Depends from fastapi.requests import Request @@ -14,7 +14,7 @@ def _get_db_engine(request: Request) -> AsyncEngine: return request.app.state.engine -def get_repository(repo_type: Type[BaseRepository]) -> Callable: +def get_repository(repo_type: type[BaseRepository]) -> Callable: async def _get_repo( engine: AsyncEngine = Depends(_get_db_engine), ) -> AsyncGenerator[BaseRepository, None]: @@ -25,7 +25,7 @@ async def _get_repo( # now the current solution is to connect connection when needed. logger.info( "%s", - f"current pool connections {engine.pool.checkedin()=},{engine.pool.checkedout()=}", + f"current pool connections {engine.pool.checkedin()=},{engine.pool.checkedout()=}", # type: ignore ) yield repo_type(db_engine=engine) diff --git a/services/catalog/src/simcore_service_catalog/api/routes/meta.py b/services/catalog/src/simcore_service_catalog/api/routes/meta.py index 23691cb6685..5d984d4cd36 100644 --- a/services/catalog/src/simcore_service_catalog/api/routes/meta.py +++ b/services/catalog/src/simcore_service_catalog/api/routes/meta.py @@ -1,7 +1,8 @@ from fastapi import APIRouter +from pydantic import parse_obj_as -from ...meta import API_VERSION, API_VTAG, __version__ -from ...models.schemas.meta import Meta +from ...meta import API_VERSION, API_VTAG +from ...models.schemas.meta import Meta, VersionStr router = APIRouter() @@ -10,6 +11,6 @@ async def get_service_metadata(): return Meta( name=__name__.split(".")[0], - version=API_VERSION, - released={API_VTAG: API_VERSION}, + version=parse_obj_as(VersionStr, API_VERSION), + released={API_VTAG: parse_obj_as(VersionStr, API_VERSION)}, ) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/groups.py b/services/catalog/src/simcore_service_catalog/db/repositories/groups.py index 0279da181b6..e93335f1095 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/groups.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/groups.py @@ -1,4 +1,4 @@ -from typing import Dict, List, Optional, Set +from typing import Optional, cast import sqlalchemy as sa from pydantic.networks import EmailStr @@ -11,7 +11,7 @@ class GroupsRepository(BaseRepository): - async def list_user_groups(self, user_id: int) -> List[GroupAtDB]: + async def list_user_groups(self, user_id: int) -> list[GroupAtDB]: groups_in_db = [] async with self.db_engine.connect() as conn: async for row in await conn.stream( @@ -21,7 +21,7 @@ async def list_user_groups(self, user_id: int) -> List[GroupAtDB]: ) .where(user_to_groups.c.uid == user_id) ): - groups_in_db.append(GroupAtDB(**row)) + groups_in_db.append(GroupAtDB.from_orm(row)) return groups_in_db async def get_everyone_group(self) -> GroupAtDB: @@ -32,31 +32,40 @@ async def get_everyone_group(self) -> GroupAtDB: row = result.first() if not row: raise RepositoryError(f"{GroupType.EVERYONE} groups was never initialized") - return GroupAtDB(**row) + return GroupAtDB.from_orm(row) async def get_user_gid_from_email( self, user_email: EmailStr ) -> Optional[PositiveInt]: async with self.db_engine.connect() as conn: - return await conn.scalar( - sa.select([users.c.primary_gid]).where(users.c.email == user_email) + return cast( + Optional[PositiveInt], + await conn.scalar( + sa.select([users.c.primary_gid]).where(users.c.email == user_email) + ), ) async def get_gid_from_affiliation(self, affiliation: str) -> Optional[PositiveInt]: async with self.db_engine.connect() as conn: - return await conn.scalar( - sa.select([groups.c.gid]).where(groups.c.name == affiliation) + return cast( + Optional[PositiveInt], + await conn.scalar( + sa.select([groups.c.gid]).where(groups.c.name == affiliation) + ), ) async def get_user_email_from_gid(self, gid: PositiveInt) -> Optional[EmailStr]: async with self.db_engine.connect() as conn: - return await conn.scalar( - sa.select([users.c.email]).where(users.c.primary_gid == gid) + return cast( + Optional[EmailStr], + await conn.scalar( + sa.select([users.c.email]).where(users.c.primary_gid == gid) + ), ) async def list_user_emails_from_gids( - self, gids: Set[PositiveInt] - ) -> Dict[PositiveInt, Optional[EmailStr]]: + self, gids: set[PositiveInt] + ) -> dict[PositiveInt, Optional[EmailStr]]: service_owners = {} async with self.db_engine.connect() as conn: async for row in await conn.stream( diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/services.py b/services/catalog/src/simcore_service_catalog/db/repositories/services.py index 366e4d82dee..1e65becee52 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/services.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/services.py @@ -139,7 +139,7 @@ async def list_service_releases( releases = [] async with self.db_engine.connect() as conn: async for row in await conn.stream(query): - releases.append(ServiceMetaDataAtDB(**row)) + releases.append(ServiceMetaDataAtDB.from_orm(row)) # Now sort naturally from latest first: (This is lame, the sorting should be done in the db) return sorted( @@ -348,7 +348,7 @@ async def get_service_specifications( self, key: ServiceKey, version: ServiceVersion, - groups: tuple[GroupAtDB], + groups: tuple[GroupAtDB, ...], allow_use_latest_service_version: bool = False, ) -> Optional[ServiceSpecifications]: """returns the service specifications for service 'key:version' and for 'groups' From 7ce15e23f563636336081f16377735b36f5c68bc Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 18:54:12 +0100 Subject: [PATCH 07/21] mypy --- .../simcore_service_catalog/api/dependencies/database.py | 4 ---- .../simcore_service_catalog/api/dependencies/services.py | 2 +- .../src/simcore_service_catalog/api/routes/dags.py | 6 +++--- .../api/routes/services_resources.py | 8 ++++---- .../src/simcore_service_catalog/core/background_tasks.py | 4 ++-- services/catalog/src/simcore_service_catalog/db/events.py | 2 +- .../catalog/src/simcore_service_catalog/utils/pools.py | 5 +++-- 7 files changed, 14 insertions(+), 17 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/api/dependencies/database.py b/services/catalog/src/simcore_service_catalog/api/dependencies/database.py index d0de2107be5..1e1af7c5046 100644 --- a/services/catalog/src/simcore_service_catalog/api/dependencies/database.py +++ b/services/catalog/src/simcore_service_catalog/api/dependencies/database.py @@ -23,10 +23,6 @@ async def _get_repo( # 2nd one was acquiring a connection per request which works but blocks the director-v2 responsiveness once # the max amount of connections is reached # now the current solution is to connect connection when needed. - logger.info( - "%s", - f"current pool connections {engine.pool.checkedin()=},{engine.pool.checkedout()=}", # type: ignore - ) yield repo_type(db_engine=engine) return _get_repo diff --git a/services/catalog/src/simcore_service_catalog/api/dependencies/services.py b/services/catalog/src/simcore_service_catalog/api/dependencies/services.py index d92c4d374ba..02bb97fee65 100644 --- a/services/catalog/src/simcore_service_catalog/api/dependencies/services.py +++ b/services/catalog/src/simcore_service_catalog/api/dependencies/services.py @@ -17,7 +17,7 @@ from ...services.director import DirectorApi from ...services.function_services import get_function_service, is_function_service from .database import get_repository -from .director import DirectorApi, get_director_api +from .director import get_director_api def get_default_service_resources(request: Request) -> ResourcesDict: diff --git a/services/catalog/src/simcore_service_catalog/api/routes/dags.py b/services/catalog/src/simcore_service_catalog/api/routes/dags.py index 054b3857a71..cf22a9d9c81 100644 --- a/services/catalog/src/simcore_service_catalog/api/routes/dags.py +++ b/services/catalog/src/simcore_service_catalog/api/routes/dags.py @@ -1,5 +1,5 @@ import logging -from typing import List, Optional +from typing import Optional from fastapi import APIRouter, Body, Depends, HTTPException, Query from starlette.status import ( @@ -17,7 +17,7 @@ log = logging.getLogger(__name__) -@router.get("", response_model=List[DAGOut]) +@router.get("", response_model=list[DAGOut]) async def list_dags( page_token: Optional[str] = Query( None, description="Requests a specific page of the list results" @@ -102,7 +102,7 @@ async def udpate_dag( dag: DAGIn = Body(None), dags_repo: DAGsRepository = Depends(get_repository(DAGsRepository)), ): - async with dags_repo.connection.begin(): + async with dags_repo.db_engine.begin(): await dags_repo.update_dag(dag_id, dag) updated_dag = await dags_repo.get_dag(dag_id) diff --git a/services/catalog/src/simcore_service_catalog/api/routes/services_resources.py b/services/catalog/src/simcore_service_catalog/api/routes/services_resources.py index 5917d844834..93b5b7f5762 100644 --- a/services/catalog/src/simcore_service_catalog/api/routes/services_resources.py +++ b/services/catalog/src/simcore_service_catalog/api/routes/services_resources.py @@ -26,20 +26,20 @@ RESPONSE_MODEL_POLICY, SIMCORE_SERVICE_SETTINGS_LABELS, ) +from ...services.director import DirectorApi from ...services.function_services import is_function_service from ...utils.service_resources import ( merge_service_resources_with_user_specs, parse_generic_resource, ) from ..dependencies.database import get_repository -from ..dependencies.director import DirectorApi, get_director_api +from ..dependencies.director import get_director_api from ..dependencies.services import get_default_service_resources from ..dependencies.user_groups import list_user_groups router = APIRouter() logger = logging.getLogger(__name__) -SIMCORE_SERVICE_SETTINGS_LABELS: Final[str] = "simcore.service.settings" SIMCORE_SERVICE_COMPOSE_SPEC_LABEL: Final[str] = "simcore.service.compose-spec" @@ -199,10 +199,10 @@ async def get_service_resources( ) if not spec_service_labels: - spec_service_resources: ResourcesDict = default_service_resources + spec_service_resources = default_service_resources else: spec_service_settings = _get_service_settings(spec_service_labels) - spec_service_resources: ResourcesDict = _resources_from_settings( + spec_service_resources = _resources_from_settings( spec_service_settings, default_service_resources, service_key, diff --git a/services/catalog/src/simcore_service_catalog/core/background_tasks.py b/services/catalog/src/simcore_service_catalog/core/background_tasks.py index 199425c69b4..81ec3c533f0 100644 --- a/services/catalog/src/simcore_service_catalog/core/background_tasks.py +++ b/services/catalog/src/simcore_service_catalog/core/background_tasks.py @@ -13,7 +13,7 @@ import logging from contextlib import suppress from pprint import pformat -from typing import Final +from typing import Any, Final, cast from fastapi import FastAPI from models_library.services import ServiceDockerData @@ -42,7 +42,7 @@ async def _list_registry_services( ) -> ServiceDockerDataMap: client = get_director_api(app) - data = await client.get("/services") + data = cast(list[dict[str, Any]], await client.get("/services")) services: ServiceDockerDataMap = { (s.key, s.version): s for s in iter_service_docker_data() } diff --git a/services/catalog/src/simcore_service_catalog/db/events.py b/services/catalog/src/simcore_service_catalog/db/events.py index 50ef822c936..3e7e075c151 100644 --- a/services/catalog/src/simcore_service_catalog/db/events.py +++ b/services/catalog/src/simcore_service_catalog/db/events.py @@ -2,6 +2,7 @@ from fastapi import FastAPI from servicelib.retry_policies import PostgresRetryPolicyUponInitialization +from settings_library.postgres import PostgresSettings from simcore_postgres_database.utils_aiosqlalchemy import ( get_pg_engine_stateinfo, raise_if_migration_not_ready, @@ -9,7 +10,6 @@ from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine from tenacity import retry -from ..core.settings import PostgresSettings from .repositories.products import ProductsRepository logger = logging.getLogger(__name__) diff --git a/services/catalog/src/simcore_service_catalog/utils/pools.py b/services/catalog/src/simcore_service_catalog/utils/pools.py index 479c81ad544..39d6f5e9801 100644 --- a/services/catalog/src/simcore_service_catalog/utils/pools.py +++ b/services/catalog/src/simcore_service_catalog/utils/pools.py @@ -1,5 +1,6 @@ from concurrent.futures import ProcessPoolExecutor from contextlib import contextmanager +from typing import Iterator # only gets created on use and is guaranteed to be the s # ame for the entire lifetime of the application @@ -9,7 +10,7 @@ def get_shared_process_pool_executor(**kwargs) -> ProcessPoolExecutor: # sometimes a pool requires a specific configuration # the key helps to distinguish between them in the same application - key = "".join(sorted(["_".join((k, str(v))) for k, v in kwargs.items()])) + key = "".join(sorted("_".join((k, str(v))) for k, v in kwargs.items())) if key not in __shared_process_pool_executor: # pylint: disable=consider-using-with @@ -21,7 +22,7 @@ def get_shared_process_pool_executor(**kwargs) -> ProcessPoolExecutor: # because there is no shared fastapi library, this is a # duplicate of servicelib.pools.non_blocking_process_pool_executor @contextmanager -def non_blocking_process_pool_executor(**kwargs) -> ProcessPoolExecutor: +def non_blocking_process_pool_executor(**kwargs) -> Iterator[ProcessPoolExecutor]: """ Avoids default context manger behavior which calls shutdown with wait=True an blocks. From 7e20a85dbc8093b93fc6a0f8753a5c1cb69d320f Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 19:04:06 +0100 Subject: [PATCH 08/21] mypy --- .../models/schemas/services_specifications.py | 3 +++ .../services/function_services.py | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/models/schemas/services_specifications.py b/services/catalog/src/simcore_service_catalog/models/schemas/services_specifications.py index 7430e417bef..000903e56d2 100644 --- a/services/catalog/src/simcore_service_catalog/models/schemas/services_specifications.py +++ b/services/catalog/src/simcore_service_catalog/models/schemas/services_specifications.py @@ -16,6 +16,9 @@ class ServiceSpecifications(BaseModel): description="schedule-time specifications specifications for the service (follows Docker Service creation API (specifically only the Resources part), see https://docs.docker.com/engine/api/v1.41/#tag/Service/operation/ServiceCreate", ) + class Config: + pass + class ServiceSpecificationsGet(ServiceSpecifications): ... diff --git a/services/catalog/src/simcore_service_catalog/services/function_services.py b/services/catalog/src/simcore_service_catalog/services/function_services.py index 74f6851c658..c8bd85f2952 100644 --- a/services/catalog/src/simcore_service_catalog/services/function_services.py +++ b/services/catalog/src/simcore_service_catalog/services/function_services.py @@ -2,7 +2,7 @@ Catalog of i/o metadata for functions implemented in the front-end """ -from typing import Any, Dict, Tuple +from typing import Any, cast from fastapi import status from fastapi.applications import FastAPI @@ -16,13 +16,13 @@ assert is_function_service # nosec -def _as_dict(model_instance: ServiceDockerData) -> Dict[str, Any]: +def _as_dict(model_instance: ServiceDockerData) -> dict[str, Any]: # FIXME: In order to convert to ServiceOut, now we have to convert back to front-end service because of alias # FIXME: set the same policy for f/e and director datasets! - return model_instance.dict(by_alias=True, exclude_unset=True) + return cast(dict[str, Any], model_instance.dict(by_alias=True, exclude_unset=True)) -def get_function_service(key, version) -> Dict[str, Any]: +def get_function_service(key, version) -> dict[str, Any]: try: found = next( s @@ -51,7 +51,7 @@ def _on_startup() -> None: app.add_event_handler("startup", _on_startup) -__all__: Tuple[str, ...] = ( +__all__: tuple[str, ...] = ( "get_function_service", "is_function_service", "setup_function_services", From a2c616af31ffca50db11e6f010edc5991fa4fecb Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 22:01:21 +0100 Subject: [PATCH 09/21] allow for sha256 usage --- packages/models-library/src/models_library/basic_regex.py | 2 +- packages/models-library/tests/test_service_resources.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/models-library/src/models_library/basic_regex.py b/packages/models-library/src/models_library/basic_regex.py index 895da8ba6d3..72561063206 100644 --- a/packages/models-library/src/models_library/basic_regex.py +++ b/packages/models-library/src/models_library/basic_regex.py @@ -71,5 +71,5 @@ DOCKER_IMAGE_KEY_RE = r"[\w/-]+" DOCKER_IMAGE_VERSION_RE = r"[\w/.]+" DOCKER_GENERIC_TAG_KEY_RE: Final[re.Pattern] = re.compile( - r"^(?P(?:(?:(?:[a-zA-Z0-9-]+\.)+[a-zA-Z0-9-]+(?::\d+)?)|[a-zA-Z0-9-]+:\d+))?(?:/)?(?P(?:[a-z0-9][a-z0-9_.-]*/)*[a-z0-9-_]+[a-z0-9])(?::(?P[\w][\w.-]{0,126}[\w]))?$" + r"^(?P(?:(?:(?:[a-zA-Z0-9-]+\.)+[a-zA-Z0-9-]+(?::\d+)?)|[a-zA-Z0-9-]+:\d+))?(?:/)?(?P(?:[a-z0-9][a-z0-9_.-]*/)*[a-z0-9-_]+[a-z0-9])(?::(?P[\w][\w.-]{0,126}[\w]))?(?P\@sha256:[a-fA-F0-9]{64})?$" ) diff --git a/packages/models-library/tests/test_service_resources.py b/packages/models-library/tests/test_service_resources.py index 25d00df884b..34300d6b277 100644 --- a/packages/models-library/tests/test_service_resources.py +++ b/packages/models-library/tests/test_service_resources.py @@ -23,7 +23,7 @@ "simcore/services/dynamic/nice-service:v1.0.0", "a/docker-hub/image:1.0.0", "traefik:v1.0.0", - "traefik:v1.0.0@somehash", + "traefik:v1.0.0@sha256:4bed291aa5efb9f0d77b76ff7d4ab71eee410962965d052552db1fb80576431d", ), ) def test_compose_image(example: str) -> None: From dc726f68d132cf7b80ca7b5a4214ab1e1b1bfe6b Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 22:06:20 +0100 Subject: [PATCH 10/21] sonarcloud --- .../catalog/src/simcore_service_catalog/models/schemas/meta.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/catalog/src/simcore_service_catalog/models/schemas/meta.py b/services/catalog/src/simcore_service_catalog/models/schemas/meta.py index 0899035ad54..3cb7da165d4 100644 --- a/services/catalog/src/simcore_service_catalog/models/schemas/meta.py +++ b/services/catalog/src/simcore_service_catalog/models/schemas/meta.py @@ -7,7 +7,7 @@ # use https://www.python.org/dev/peps/pep-0440/#version-scheme # or https://www.python.org/dev/peps/pep-0440/#appendix-b-parsing-version-strings-with-regular-expressions # -VERSION_RE = r"^(0|[1-9]\d*)(\.(0|[1-9]\d*)){2}(-(0|[1-9]\d*|\d*[-a-zA-Z][-\da-zA-Z]*)(\.(0|[1-9]\d*|\d*[-a-zA-Z][-\da-zA-Z]*))*)?(\+[-\da-zA-Z]+(\.[-\da-zA-Z-]+)*)?$" +VERSION_RE = r"^(0|[1-9]\d*)(\.(0|[1-9]\d*)){2}(-(0|[1-9]\d*|\d*[-a-zA-Z][-\da-zA-Z]*)(\.(0|[1-9]\d*|\d*[-a-zA-Z][-\da-zA-Z]*))*)?(\+[-\da-zA-Z]+(\.[-\da-zA-Z]+)*)?$" class VersionStr(ConstrainedStr): From c7a869206032f90482a026271d42fc22eacb8a5c Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Thu, 16 Mar 2023 22:33:33 +0100 Subject: [PATCH 11/21] fix --- .../src/simcore_service_catalog/db/repositories/groups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/groups.py b/services/catalog/src/simcore_service_catalog/db/repositories/groups.py index e93335f1095..5df177b1420 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/groups.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/groups.py @@ -29,7 +29,7 @@ async def get_everyone_group(self) -> GroupAtDB: result = await conn.execute( sa.select([groups]).where(groups.c.type == GroupType.EVERYONE) ) - row = result.first() + row = await result.first() if not row: raise RepositoryError(f"{GroupType.EVERYONE} groups was never initialized") return GroupAtDB.from_orm(row) From be956a7f336571bcdc0a91e64c175bfedf23a45e Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Mar 2023 10:15:44 +0100 Subject: [PATCH 12/21] added mypy in sqlalchemy --- services/catalog/requirements/_base.txt | 9 +++++---- services/catalog/requirements/_test.in | 1 + services/catalog/requirements/_test.txt | 14 +++++++++++++- services/catalog/requirements/_tools.txt | 6 ++++-- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/services/catalog/requirements/_base.txt b/services/catalog/requirements/_base.txt index c31a2d1dc35..4d174f6cddf 100644 --- a/services/catalog/requirements/_base.txt +++ b/services/catalog/requirements/_base.txt @@ -1,6 +1,6 @@ # -# This file is autogenerated by pip-compile with python 3.9 -# To update, run: +# This file is autogenerated by pip-compile with Python 3.9 +# by the following command: # # pip-compile --output-file=requirements/_base.txt --resolver=backtracking --strip-extras requirements/_base.in # @@ -148,7 +148,9 @@ pyyaml==5.4.1 # fastapi # uvicorn redis==4.4.0 - # via -r requirements/../../../packages/service-library/requirements/_base.in + # via + # -c requirements/../../../packages/service-library/requirements/./_base.in + # -r requirements/../../../packages/service-library/requirements/_base.in requests==2.27.1 # via fastapi rfc3986==1.4.0 @@ -166,7 +168,6 @@ sniffio==1.2.0 sqlalchemy==1.4.37 # via # -r requirements/../../../packages/postgres-database/requirements/_base.in - # -r requirements/_base.in # alembic starlette==0.20.4 # via fastapi diff --git a/services/catalog/requirements/_test.in b/services/catalog/requirements/_test.in index 1c7ce62e888..079b52ae51f 100644 --- a/services/catalog/requirements/_test.in +++ b/services/catalog/requirements/_test.in @@ -25,3 +25,4 @@ pytest-docker pytest-mock pytest-runner respx +sqlalchemy[asyncio, mypy] diff --git a/services/catalog/requirements/_test.txt b/services/catalog/requirements/_test.txt index 240ec829039..a18942d72b0 100644 --- a/services/catalog/requirements/_test.txt +++ b/services/catalog/requirements/_test.txt @@ -101,6 +101,10 @@ multidict==6.0.2 # -c requirements/_base.txt # aiohttp # yarl +mypy==1.1.1 + # via sqlalchemy +mypy-extensions==1.0.0 + # via mypy packaging==21.3 # via # -c requirements/_base.txt @@ -170,12 +174,20 @@ sniffio==1.2.0 # httpx sqlalchemy==1.4.37 # via - # -c requirements/_base.txt + # -r requirements/_test.in # alembic +sqlalchemy2-stubs==0.0.2a32 + # via sqlalchemy tomli==2.0.1 # via # coverage + # mypy # pytest +typing-extensions==4.3.0 + # via + # -c requirements/_base.txt + # mypy + # sqlalchemy2-stubs urllib3==1.26.9 # via # -c requirements/_base.txt diff --git a/services/catalog/requirements/_tools.txt b/services/catalog/requirements/_tools.txt index c4651eef89d..96ec943c6ae 100644 --- a/services/catalog/requirements/_tools.txt +++ b/services/catalog/requirements/_tools.txt @@ -36,7 +36,9 @@ lazy-object-proxy==1.9.0 mccabe==0.7.0 # via pylint mypy-extensions==1.0.0 - # via black + # via + # -c requirements/_test.txt + # black nodeenv==1.7.0 # via pre-commit packaging==21.3 @@ -78,7 +80,7 @@ tomlkit==0.11.6 # via pylint typing-extensions==4.3.0 # via - # -c requirements/_base.txt + # -c requirements/_test.txt # astroid # black # pylint From 54be35af4b3be670ca4e6b24a8ac8ad7282108cf Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Mar 2023 10:33:24 +0100 Subject: [PATCH 13/21] mypy --- .../catalog/src/simcore_service_catalog/api/routes/services.py | 3 ++- .../catalog/src/simcore_service_catalog/core/application.py | 3 ++- services/catalog/src/simcore_service_catalog/core/events.py | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/api/routes/services.py b/services/catalog/src/simcore_service_catalog/api/routes/services.py index aeaf5daf1d8..4def397a34b 100644 --- a/services/catalog/src/simcore_service_catalog/api/routes/services.py +++ b/services/catalog/src/simcore_service_catalog/api/routes/services.py @@ -21,10 +21,11 @@ RESPONSE_MODEL_POLICY, ) from ...models.schemas.services import ServiceGet, ServiceUpdate +from ...services.director import DirectorApi from ...services.function_services import is_function_service from ...utils.requests_decorators import cancellable_request from ..dependencies.database import get_repository -from ..dependencies.director import DirectorApi, get_director_api +from ..dependencies.director import get_director_api from ..dependencies.services import get_service_from_registry logger = logging.getLogger(__name__) diff --git a/services/catalog/src/simcore_service_catalog/core/application.py b/services/catalog/src/simcore_service_catalog/core/application.py index 9738689b9d5..46045a81c1d 100644 --- a/services/catalog/src/simcore_service_catalog/core/application.py +++ b/services/catalog/src/simcore_service_catalog/core/application.py @@ -5,6 +5,7 @@ from fastapi import FastAPI, Request from fastapi.exceptions import RequestValidationError from fastapi.middleware.gzip import GZipMiddleware +from models_library.basic_types import BootModeEnum from servicelib.fastapi.openapi import override_fastapi_openapi_method from starlette import status from starlette.exceptions import HTTPException @@ -25,7 +26,7 @@ on_shutdown, on_startup, ) -from .settings import AppSettings, BootModeEnum +from .settings import AppSettings logger = logging.getLogger(__name__) diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index cb0b64fb4f1..45d2084bece 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -2,6 +2,7 @@ from typing import Callable from fastapi import FastAPI +from models_library.basic_types import BootModeEnum from servicelib.fastapi.tracing import setup_tracing from ..db.events import close_db_connection, connect_to_db, setup_default_product @@ -9,7 +10,6 @@ from ..services.director import close_director, setup_director from ..services.remote_debug import setup_remote_debugging from .background_tasks import start_registry_sync_task, stop_registry_sync_task -from .settings import BootModeEnum logger = logging.getLogger(__name__) From b4992f04588eba24fd7a74bf6269e6cdcf98fc3b Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Mar 2023 10:35:04 +0100 Subject: [PATCH 14/21] first is not awaitable --- .../src/simcore_service_catalog/db/repositories/groups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/groups.py b/services/catalog/src/simcore_service_catalog/db/repositories/groups.py index 5df177b1420..e93335f1095 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/groups.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/groups.py @@ -29,7 +29,7 @@ async def get_everyone_group(self) -> GroupAtDB: result = await conn.execute( sa.select([groups]).where(groups.c.type == GroupType.EVERYONE) ) - row = await result.first() + row = result.first() if not row: raise RepositoryError(f"{GroupType.EVERYONE} groups was never initialized") return GroupAtDB.from_orm(row) From 30b6dec8de2898406a3000cf6b8024ada705b3a4 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Mar 2023 10:35:23 +0100 Subject: [PATCH 15/21] mypy --- .../db/repositories/dags.py | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/dags.py b/services/catalog/src/simcore_service_catalog/db/repositories/dags.py index 47efde0e936..973d5b9c4f2 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/dags.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/dags.py @@ -1,5 +1,5 @@ import json -from typing import List, Optional +from typing import Optional import sqlalchemy as sa @@ -10,7 +10,7 @@ class DAGsRepository(BaseRepository): - async def list_dags(self) -> List[DAGAtDB]: + async def list_dags(self) -> list[DAGAtDB]: dagraphs = [] async with self.db_engine.connect() as conn: async for row in await conn.stream(dags.select()): @@ -19,23 +19,23 @@ async def list_dags(self) -> List[DAGAtDB]: async def get_dag(self, dag_id: int) -> Optional[DAGAtDB]: async with self.db_engine.connect() as conn: - row = await conn.execute(dags.select().where(dags.c.id == dag_id)).first() + result = await conn.execute(dags.select().where(dags.c.id == dag_id)) + row = result.first() if row: - return DAGAtDB(**row) + return DAGAtDB.from_orm(row) async def create_dag(self, dag: DAGIn) -> int: async with self.db_engine.begin() as conn: - new_id: int = await ( - await conn.execute( - dags.insert().values( - workbench=dag.json(include={"workbench"}), - **dag.dict(exclude={"workbench"}) - ) + new_id: int = await conn.scalar( + dags.insert().values( + workbench=dag.json(include={"workbench"}), + **dag.dict(exclude={"workbench"}) ) - ).scalar() + ) + return new_id - async def replace_dag(self, dag_id: int, dag: DAGIn): + async def replace_dag(self, dag_id: int, dag: DAGIn) -> None: async with self.db_engine.begin() as conn: await conn.execute( dags.update() @@ -46,18 +46,15 @@ async def replace_dag(self, dag_id: int, dag: DAGIn): .where(dags.c.id == dag_id) ) - async def update_dag(self, dag_id: int, dag: DAGIn): + async def update_dag(self, dag_id: int, dag: DAGIn) -> None: patch = dag.dict(exclude_unset=True, exclude={"workbench"}) if "workbench" in dag.__fields_set__: patch["workbench"] = json.dumps(patch["workbench"]) async with self.db_engine.begin() as conn: - res = await conn.execute( + await conn.execute( sa.update(dags).values(**patch).where(dags.c.id == dag_id) ) - # TODO: dev asserts - assert res.returns_rows == False # nosec - - async def delete_dag(self, dag_id: int): + async def delete_dag(self, dag_id: int) -> None: async with self.db_engine.begin() as conn: await conn.execute(sa.delete(dags).where(dags.c.id == dag_id)) From d19ffcd153e666f4c25f510a25c2dcc81621b6ab Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Mar 2023 10:43:43 +0100 Subject: [PATCH 16/21] mypy --- .../src/simcore_service_catalog/api/dependencies/director.py | 3 ++- .../src/simcore_service_catalog/db/repositories/dags.py | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/services/catalog/src/simcore_service_catalog/api/dependencies/director.py b/services/catalog/src/simcore_service_catalog/api/dependencies/director.py index e47d22eb663..f55b55ac68e 100644 --- a/services/catalog/src/simcore_service_catalog/api/dependencies/director.py +++ b/services/catalog/src/simcore_service_catalog/api/dependencies/director.py @@ -11,4 +11,5 @@ def _get_app(request: Request) -> FastAPI: def get_director_api( app: FastAPI = Depends(_get_app), ) -> DirectorApi: - return app.state.director_api + director: DirectorApi = app.state.director_api + return director diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/dags.py b/services/catalog/src/simcore_service_catalog/db/repositories/dags.py index 973d5b9c4f2..d381d0b25fe 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/dags.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/dags.py @@ -23,6 +23,7 @@ async def get_dag(self, dag_id: int) -> Optional[DAGAtDB]: row = result.first() if row: return DAGAtDB.from_orm(row) + return None async def create_dag(self, dag: DAGIn) -> int: async with self.db_engine.begin() as conn: From 518a712ea597635a98a31525fef910d84c57ebe1 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Mar 2023 13:51:19 +0100 Subject: [PATCH 17/21] mypy --- .../api/dependencies/database.py | 2 +- .../db/repositories/services.py | 22 ++++++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/api/dependencies/database.py b/services/catalog/src/simcore_service_catalog/api/dependencies/database.py index 1e1af7c5046..ed12ca5afa1 100644 --- a/services/catalog/src/simcore_service_catalog/api/dependencies/database.py +++ b/services/catalog/src/simcore_service_catalog/api/dependencies/database.py @@ -5,7 +5,7 @@ from fastapi.requests import Request from sqlalchemy.ext.asyncio import AsyncEngine -from ...db.repositories import BaseRepository +from ...db.repositories._base import BaseRepository logger = logging.getLogger(__name__) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/services.py b/services/catalog/src/simcore_service_catalog/db/repositories/services.py index 1e65becee52..19d1a19574e 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/services.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/services.py @@ -1,7 +1,8 @@ import logging from collections import defaultdict +from distutils.version import Version from itertools import chain -from typing import Any, Iterable, Optional +from typing import Any, Iterable, Optional, cast import packaging.version import sqlalchemy as sa @@ -97,7 +98,7 @@ async def list_services( product_name, ) ): - services_in_db.append(ServiceMetaDataAtDB(**row)) + services_in_db.append(ServiceMetaDataAtDB.from_orm(row)) return services_in_db async def list_service_releases( @@ -142,9 +143,11 @@ async def list_service_releases( releases.append(ServiceMetaDataAtDB.from_orm(row)) # Now sort naturally from latest first: (This is lame, the sorting should be done in the db) - return sorted( - releases, key=lambda x: packaging.version.parse(x.version), reverse=True - ) + def _by_version(x: ServiceMetaDataAtDB) -> Version: + return cast(Version, packaging.version.parse(x.version)) + + releases_sorted = sorted(releases, key=_by_version, reverse=True) + return releases_sorted async def get_latest_release(self, key: str) -> Optional[ServiceMetaDataAtDB]: """Returns last release or None if service was never released""" @@ -190,7 +193,8 @@ async def get_service( result = await conn.execute(query) row = result.first() if row: - return ServiceMetaDataAtDB(**row) + return ServiceMetaDataAtDB.from_orm(row) + return None # mypy async def create_service( self, @@ -216,7 +220,7 @@ async def create_service( ) row = result.first() assert row # nosec - created_service = ServiceMetaDataAtDB(**row) + created_service = ServiceMetaDataAtDB.from_orm(row) for access_rights in new_service_access_rights: insert_stmt = pg_insert(services_access_rights).values( @@ -242,7 +246,7 @@ async def update_service( ) row = result.first() assert row # nosec - updated_service = ServiceMetaDataAtDB(**row) + updated_service = ServiceMetaDataAtDB.from_orm(row) return updated_service async def get_service_access_rights( @@ -360,6 +364,7 @@ async def get_service_specifications( "getting specifications from db for %s", f"{key}:{version} for {groups=}" ) gid_to_group_map = {group.gid: group for group in groups} + group_specs = { GroupType.EVERYONE: None, GroupType.PRIMARY: None, @@ -418,6 +423,7 @@ async def get_service_specifications( group_specs[GroupType.PRIMARY], ): return ServiceSpecifications.parse_obj(merged_specifications) + return None # mypy def _is_newer( From 95f541053682f28e3bed470d49692a643c5c5d87 Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Mar 2023 13:57:12 +0100 Subject: [PATCH 18/21] mypy --- .../src/simcore_postgres_database/utils_products.py | 4 ++-- .../src/simcore_service_catalog/core/background_tasks.py | 7 +++++-- .../simcore_service_catalog/db/repositories/products.py | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_products.py b/packages/postgres-database/src/simcore_postgres_database/utils_products.py index f4536eeb58c..b05e699b8a4 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_products.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_products.py @@ -2,7 +2,7 @@ """ -from typing import Optional, Protocol +from typing import Any, Optional, Protocol import sqlalchemy as sa @@ -23,7 +23,7 @@ async def scalar(self, *args, **kwargs): class _AiopgConnection(Protocol): # Prototype to account for aiopg-only (this protocol avoids import <-> installation) - async def scalar(self, *args, **kwargs): + async def scalar(self, *args, **kwargs) -> Any: ... async def execute(self, *args, **kwargs): diff --git a/services/catalog/src/simcore_service_catalog/core/background_tasks.py b/services/catalog/src/simcore_service_catalog/core/background_tasks.py index 81ec3c533f0..e6adf6c074b 100644 --- a/services/catalog/src/simcore_service_catalog/core/background_tasks.py +++ b/services/catalog/src/simcore_service_catalog/core/background_tasks.py @@ -16,6 +16,7 @@ from typing import Any, Final, cast from fastapi import FastAPI +from models_library.function_services_catalog.api import iter_service_docker_data from models_library.services import ServiceDockerData from models_library.services_db import ServiceAccessRightsAtDB, ServiceMetaDataAtDB from packaging.version import Version @@ -27,7 +28,6 @@ from ..db.repositories.projects import ProjectsRepository from ..db.repositories.services import ServicesRepository from ..services import access_rights -from ..services.function_services import iter_service_docker_data logger = logging.getLogger(__name__) @@ -83,7 +83,10 @@ async def _create_services_in_db( services_repo = ServicesRepository(app.state.engine) - sorted_services = sorted(service_keys, key=lambda t: Version(t[1])) + def _by_version(t: tuple[ServiceKey, ServiceVersion]) -> Version: + return Version(t[1]) + + sorted_services = sorted(service_keys, key=_by_version) for service_key, service_version in sorted_services: service_metadata: ServiceDockerData = services_in_registry[ diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/products.py b/services/catalog/src/simcore_service_catalog/db/repositories/products.py index 21a2bc02df0..57b036150d2 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/products.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/products.py @@ -6,5 +6,5 @@ class ProductsRepository(BaseRepository): async def get_default_product_name(self) -> str: async with self.db_engine.begin() as conn: - product_name = await get_default_product_name(conn) + product_name: str = await get_default_product_name(conn) return product_name From 18385d78e362ff7dceb7833b8b5e807c24bc3e2b Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Mar 2023 14:09:09 +0100 Subject: [PATCH 19/21] mypy --- .../services/access_rights.py | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/services/access_rights.py b/services/catalog/src/simcore_service_catalog/services/access_rights.py index 6a6809e0335..037ab20121f 100644 --- a/services/catalog/src/simcore_service_catalog/services/access_rights.py +++ b/services/catalog/src/simcore_service_catalog/services/access_rights.py @@ -4,7 +4,7 @@ import logging import operator from datetime import datetime -from typing import Callable, Optional, Union +from typing import Any, Callable, Optional, Union, cast from urllib.parse import quote_plus from fastapi import FastAPI @@ -31,8 +31,11 @@ def _is_frontend_service(service: ServiceDockerData) -> bool: async def _is_old_service(app: FastAPI, service: ServiceDockerData) -> bool: # get service build date client = get_director_api(app) - data = await client.get( - f"/service_extras/{quote_plus(service.key)}/{service.version}" + data = cast( + dict[str, Any], + await client.get( + f"/service_extras/{quote_plus(service.key)}/{service.version}" + ), ) if not data or "build_date" not in data: return True @@ -161,9 +164,10 @@ def get_target(access: ServiceAccessRightsAtDB) -> tuple[Union[str, int], ...]: def get_flags(access: ServiceAccessRightsAtDB) -> dict[str, bool]: """Extracts only""" - return access.dict(include={"execute_access", "write_access"}) + flags = access.dict(include={"execute_access", "write_access"}) + return cast(dict[str, bool], flags) - access_flags_map = {} + access_flags_map: dict[tuple[Union[str, int], ...], dict[str, bool]] = {} for access in access_rights: target = get_target(access) access_flags = access_flags_map.get(target) @@ -179,10 +183,10 @@ def get_flags(access: ServiceAccessRightsAtDB) -> dict[str, bool]: for target in access_flags_map: reduced_access_rights.append( ServiceAccessRightsAtDB( - key=target[0], - version=target[1], - gid=target[2], - product_name=target[3], + key=f"{target[0]}", + version=f"{target[1]}", + gid=int(target[2]), + product_name=f"{target[3]}", **access_flags_map[target], ) ) From 1e9c5c819debfb08cca61184aae7091db3d262ac Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Mar 2023 14:16:10 +0100 Subject: [PATCH 20/21] mypy --- .../db/repositories/services.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/services.py b/services/catalog/src/simcore_service_catalog/db/repositories/services.py index 19d1a19574e..c0b01051476 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/services.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/services.py @@ -365,11 +365,9 @@ async def get_service_specifications( ) gid_to_group_map = {group.gid: group for group in groups} - group_specs = { - GroupType.EVERYONE: None, - GroupType.PRIMARY: None, - GroupType.STANDARD: {}, - } + everyone_specs = None + primary_specs = None + teams_specs: dict[GroupID, ServiceSpecificationsAtDB] = {} queried_version = packaging.version.parse(version) # we should instead use semver enabled postgres [https://pgxn.org/dist/semver/doc/semver.html] @@ -401,14 +399,18 @@ async def get_service_specifications( # filter by group type group = gid_to_group_map[row.gid] if (group.group_type == GroupType.STANDARD) and _is_newer( - group_specs[group.group_type].get(db_service_spec.gid), + teams_specs.get(db_service_spec.gid), db_service_spec, ): - group_specs[group.group_type][ - db_service_spec.gid - ] = db_service_spec - elif _is_newer(group_specs[group.group_type], db_service_spec): - group_specs[group.group_type] = db_service_spec + teams_specs[db_service_spec.gid] = db_service_spec + elif (group.group_type == GroupType.EVERYONE) and _is_newer( + everyone_specs, db_service_spec + ): + everyone_specs = db_service_spec + elif (group.group_type == GroupType.PRIMARY) and _is_newer( + primary_specs, db_service_spec + ): + primary_specs = db_service_spec except ValidationError as exc: logger.warning( @@ -418,9 +420,7 @@ async def get_service_specifications( ) if merged_specifications := _merge_specs( - group_specs[GroupType.EVERYONE], - group_specs[GroupType.STANDARD], - group_specs[GroupType.PRIMARY], + everyone_specs, teams_specs, primary_specs ): return ServiceSpecifications.parse_obj(merged_specifications) return None # mypy From f760f1de681bde8e24674918286353dba35a297b Mon Sep 17 00:00:00 2001 From: sanderegg <35365065+sanderegg@users.noreply.github.com> Date: Fri, 17 Mar 2023 14:30:52 +0100 Subject: [PATCH 21/21] linter --- .../simcore_service_catalog/db/repositories/services.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/db/repositories/services.py b/services/catalog/src/simcore_service_catalog/db/repositories/services.py index c0b01051476..a30ce8d6905 100644 --- a/services/catalog/src/simcore_service_catalog/db/repositories/services.py +++ b/services/catalog/src/simcore_service_catalog/db/repositories/services.py @@ -1,6 +1,5 @@ import logging from collections import defaultdict -from distutils.version import Version from itertools import chain from typing import Any, Iterable, Optional, cast @@ -143,8 +142,8 @@ async def list_service_releases( releases.append(ServiceMetaDataAtDB.from_orm(row)) # Now sort naturally from latest first: (This is lame, the sorting should be done in the db) - def _by_version(x: ServiceMetaDataAtDB) -> Version: - return cast(Version, packaging.version.parse(x.version)) + def _by_version(x: ServiceMetaDataAtDB) -> packaging.version.Version: + return cast(packaging.version.Version, packaging.version.parse(x.version)) releases_sorted = sorted(releases, key=_by_version, reverse=True) return releases_sorted @@ -269,7 +268,7 @@ async def get_service_access_rights( async with self.db_engine.connect() as conn: async for row in await conn.stream(query): - services_in_db.append(ServiceAccessRightsAtDB(**row)) + services_in_db.append(ServiceAccessRightsAtDB.from_orm(row)) return services_in_db async def list_services_access_rights( @@ -298,7 +297,7 @@ async def list_services_access_rights( row[services_access_rights.c.key], row[services_access_rights.c.version], ) - ].append(ServiceAccessRightsAtDB(**row)) + ].append(ServiceAccessRightsAtDB.from_orm(row)) return service_to_access_rights async def upsert_service_access_rights(