From 3f5d9437fd8394d4ab6f909c7ef15136d1ff9688 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9C=D0=B0=D1=80=D1=82=D1=8B=D0=BD=D0=BE=D0=B2=20=D0=9C?= =?UTF-8?q?=D0=B0=D0=BA=D1=81=D0=B8=D0=BC=20=D0=A1=D0=B5=D1=80=D0=B3=D0=B5?= =?UTF-8?q?=D0=B5=D0=B2=D0=B8=D1=87?= Date: Mon, 1 Apr 2024 17:13:21 +0300 Subject: [PATCH] [DOP-14540] Fix Pydantic v2 warnings --- .github/workflows/changelog.yml | 1 - .github/workflows/codeql-analysis.yml | 1 - .github/workflows/release.yml | 7 +- .github/workflows/test.yml | 2 +- .../changelog/next_release/47.improvement.rst | 1 + horizon/backend/api/v1/router/hwm.py | 1 + horizon/backend/api/v1/router/namespaces.py | 2 + horizon/backend/api/v1/router/users.py | 2 + horizon/client/base.py | 12 ++- horizon/commons/errors/base.py | 6 +- horizon/commons/errors/schemas/__init__.py | 1 + .../commons/errors/schemas/invalid_request.py | 8 +- horizon/commons/schemas/v1/hwm.py | 12 +-- horizon/commons/schemas/v1/hwm_history.py | 9 +- horizon/commons/schemas/v1/namespace.py | 12 +-- .../commons/schemas/v1/namespace_history.py | 9 +- horizon/commons/schemas/v1/pagination.py | 7 +- horizon/commons/schemas/v1/permission.py | 5 +- horizon/commons/schemas/v1/user.py | 9 +- pyproject.toml | 1 + tests/test_backend/test_hwm/test_copy_hmws.py | 77 +++++++++++++--- .../test_backend/test_hwm/test_create_hwm.py | 2 +- .../test_update_permissions.py | 90 ++++++++++--------- 23 files changed, 183 insertions(+), 94 deletions(-) create mode 100644 docs/changelog/next_release/47.improvement.rst diff --git a/.github/workflows/changelog.yml b/.github/workflows/changelog.yml index 7d332720..c357e155 100644 --- a/.github/workflows/changelog.yml +++ b/.github/workflows/changelog.yml @@ -42,7 +42,6 @@ jobs: ${{ runner.os }}-python-${{ env.DEFAULT_PYTHON }}-changelog-${{ hashFiles('**/poetry.lock') }} ${{ runner.os }}-python-${{ env.DEFAULT_PYTHON }}-changelog- ${{ runner.os }}-python - ${{ runner.os }}- - name: Install dependencies run: | diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 2fa99441..fc464951 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -51,7 +51,6 @@ jobs: ${{ runner.os }}-python-${{ env.DEFAULT_PYTHON }}-codeql-${{ hashFiles('**/poetry.lock') }} ${{ runner.os }}-python-${{ env.DEFAULT_PYTHON }}-codeql- ${{ runner.os }}-python - ${{ runner.os }}- - name: Install dependencies run: | diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 016a919b..df73add1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -42,10 +42,11 @@ jobs: uses: actions/cache@v4 with: path: ~/.cache/pypoetry - key: pypi-release-${{ runner.os }}-py-${{ hashFiles('**/poetry.lock') }}-${{ env.DEFAULT_PYTHON }} + key: ${{ runner.os }}-python-${{ env.DEFAULT_PYTHON }}-release-${{ hashFiles('**/poetry.lock') }} restore-keys: | - pypi-release-${{ runner.os }}-py-${{ hashFiles('**/poetry.lock') }}- - pypi-release-${{ runner.os }}-py- + ${{ runner.os }}-python-${{ env.DEFAULT_PYTHON }}-release-${{ hashFiles('**/poetry.lock') }} + ${{ runner.os }}-python-${{ env.DEFAULT_PYTHON }}-release- + ${{ runner.os }}-python - name: Install dependencies run: | diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2232b90c..a33be28c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -82,7 +82,7 @@ jobs: run: | poetry install --no-root --all-extras --with test --without dev,docs if [ "${{ matrix.pydantic-version }}" == "1" ]; then - pip install "pydantic<2.0.0" + poetry run pip install "pydantic<2.0.0" fi - name: Run Tests diff --git a/docs/changelog/next_release/47.improvement.rst b/docs/changelog/next_release/47.improvement.rst new file mode 100644 index 00000000..0891cee1 --- /dev/null +++ b/docs/changelog/next_release/47.improvement.rst @@ -0,0 +1 @@ +Fix Pydantic v2 model warnings while starting backend. diff --git a/horizon/backend/api/v1/router/hwm.py b/horizon/backend/api/v1/router/hwm.py index f3522465..2ca545fc 100644 --- a/horizon/backend/api/v1/router/hwm.py +++ b/horizon/backend/api/v1/router/hwm.py @@ -1,6 +1,7 @@ # SPDX-FileCopyrightText: 2023-2024 MTS (Mobile Telesystems) # SPDX-License-Identifier: Apache-2.0 +# mypy: disable-error-code="pydantic-orm" from fastapi import APIRouter, Depends, status from typing_extensions import Annotated diff --git a/horizon/backend/api/v1/router/namespaces.py b/horizon/backend/api/v1/router/namespaces.py index 9abce641..5f21a5e7 100644 --- a/horizon/backend/api/v1/router/namespaces.py +++ b/horizon/backend/api/v1/router/namespaces.py @@ -1,6 +1,8 @@ # SPDX-FileCopyrightText: 2023-2024 MTS (Mobile Telesystems) # SPDX-License-Identifier: Apache-2.0 +# mypy: disable-error-code="pydantic-orm" + from fastapi import APIRouter, Depends, HTTPException, status from typing_extensions import Annotated diff --git a/horizon/backend/api/v1/router/users.py b/horizon/backend/api/v1/router/users.py index b7caeb18..c1a25e19 100644 --- a/horizon/backend/api/v1/router/users.py +++ b/horizon/backend/api/v1/router/users.py @@ -1,6 +1,8 @@ # SPDX-FileCopyrightText: 2023-2024 MTS (Mobile Telesystems) # SPDX-License-Identifier: Apache-2.0 +# mypy: disable-error-code="pydantic-orm" + from typing import Union from fastapi import APIRouter, Depends diff --git a/horizon/client/base.py b/horizon/client/base.py index 487a7b5b..1a737700 100644 --- a/horizon/client/base.py +++ b/horizon/client/base.py @@ -12,7 +12,11 @@ from pydantic import AnyHttpUrl, BaseModel, PrivateAttr, ValidationError from pydantic import __version__ as pydantic_version from pydantic import parse_obj_as, validator -from pydantic.generics import GenericModel + +if pydantic_version >= "2": + from pydantic import BaseModel as GenericModel +else: + from pydantic.generics import GenericModel # type: ignore[no-redef] # noqa: WPS440 from typing_extensions import Protocol import horizon @@ -58,9 +62,9 @@ class Config: @classmethod def session_class(cls) -> type[SessionClass]: # Get `Session` from `SyncClient(BaseClient[Session])` - if pydantic_version < "2": - return cls.__bases__[0].__annotations__["session"].__args__[0] - return cls.model_fields["session"].annotation.__args__[0] + if pydantic_version >= "2": + return cls.model_fields["session"].annotation.__args__[0] # type: ignore[union-attr] + return cls.__bases__[0].__annotations__["session"].__args__[0] @validator("base_url") def _validate_url(cls, value: AnyHttpUrl): diff --git a/horizon/commons/errors/base.py b/horizon/commons/errors/base.py index b58be54d..9628e43a 100644 --- a/horizon/commons/errors/base.py +++ b/horizon/commons/errors/base.py @@ -5,7 +5,11 @@ from pydantic import BaseModel from pydantic import __version__ as pydantic_version -from pydantic.generics import GenericModel + +if pydantic_version >= "2": + from pydantic import BaseModel as GenericModel +else: + from pydantic.generics import GenericModel # type: ignore[no-redef] # noqa: WPS440 from horizon.commons.dto.unset import Unset diff --git a/horizon/commons/errors/schemas/__init__.py b/horizon/commons/errors/schemas/__init__.py index c5223e4a..9a74ee02 100644 --- a/horizon/commons/errors/schemas/__init__.py +++ b/horizon/commons/errors/schemas/__init__.py @@ -13,4 +13,5 @@ "NotAuthorizedSchema", "NotFoundSchema", "BadRequestError", + "PermissionDeniedSchema", ] diff --git a/horizon/commons/errors/schemas/invalid_request.py b/horizon/commons/errors/schemas/invalid_request.py index 27ed735a..338c3e9d 100644 --- a/horizon/commons/errors/schemas/invalid_request.py +++ b/horizon/commons/errors/schemas/invalid_request.py @@ -22,10 +22,10 @@ class InvalidRequestBaseErrorSchema(BaseModel): input: Any = Field(default=None) class Config: - # pydantic v1 - allow_population_by_field_name = True - # pydantic v2 - populate_by_name = True + if pydantic_version >= "2": + populate_by_name = True + else: + allow_population_by_field_name = True @register_error_response( diff --git a/horizon/commons/schemas/v1/hwm.py b/horizon/commons/schemas/v1/hwm.py index b9afb677..6fa895f1 100644 --- a/horizon/commons/schemas/v1/hwm.py +++ b/horizon/commons/schemas/v1/hwm.py @@ -3,7 +3,9 @@ from datetime import datetime from typing import Any, List, Optional, Union -from pydantic import BaseModel, Field, root_validator, validator +from pydantic import BaseModel, Field +from pydantic import __version__ as pydantic_version +from pydantic import root_validator, validator from horizon.commons.dto import Unset from horizon.commons.schemas.v1.pagination import PaginateQueryV1 @@ -30,10 +32,10 @@ class HWMResponseV1(BaseModel): changed_by: Optional[str] = Field(default=None, description="Latest user who changed the HWM data") class Config: - # pydantic v1 - orm_mode = True - # pydantic v2 - from_attributes = True + if pydantic_version >= "2": + from_attributes = True + else: + orm_mode = True class HWMListResponseV1(BaseModel): diff --git a/horizon/commons/schemas/v1/hwm_history.py b/horizon/commons/schemas/v1/hwm_history.py index 3479b421..e2738e1f 100644 --- a/horizon/commons/schemas/v1/hwm_history.py +++ b/horizon/commons/schemas/v1/hwm_history.py @@ -5,6 +5,7 @@ from typing import Any, Optional from pydantic import BaseModel, Field +from pydantic import __version__ as pydantic_version from horizon.commons.schemas.v1.pagination import PaginateQueryV1 @@ -37,7 +38,7 @@ class HWMHistoryResponseV1(BaseModel): changed_by: Optional[str] = Field(default=None, description="User who changed the HWM data") class Config: - # pydantic v1 - orm_mode = True - # pydantic v2 - from_attributes = True + if pydantic_version >= "2": + from_attributes = True + else: + orm_mode = True diff --git a/horizon/commons/schemas/v1/namespace.py b/horizon/commons/schemas/v1/namespace.py index bb69ca68..28a224c0 100644 --- a/horizon/commons/schemas/v1/namespace.py +++ b/horizon/commons/schemas/v1/namespace.py @@ -4,7 +4,9 @@ from enum import Enum from typing import Optional, Union -from pydantic import BaseModel, Field, root_validator +from pydantic import BaseModel, Field +from pydantic import __version__ as pydantic_version +from pydantic import root_validator from horizon.commons.dto import Unset from horizon.commons.schemas.v1.pagination import PaginateQueryV1 @@ -29,10 +31,10 @@ class NamespaceResponseV1(BaseModel): changed_by: Optional[str] = Field(default=None, description="Latest user who changed the namespace data") class Config: - # pydantic v1 - orm_mode = True - # pydantic v2 - from_attributes = True + if pydantic_version >= "2": + from_attributes = True + else: + orm_mode = True class NamespacePaginateQueryV1(PaginateQueryV1): diff --git a/horizon/commons/schemas/v1/namespace_history.py b/horizon/commons/schemas/v1/namespace_history.py index 4bc37aca..3ca38949 100644 --- a/horizon/commons/schemas/v1/namespace_history.py +++ b/horizon/commons/schemas/v1/namespace_history.py @@ -5,6 +5,7 @@ from typing import Optional from pydantic import BaseModel, Field +from pydantic import __version__ as pydantic_version from horizon.commons.schemas.v1.pagination import PaginateQueryV1 @@ -30,7 +31,7 @@ class NamespaceHistoryResponseV1(BaseModel): changed_by: Optional[str] = Field(default=None, description="User who changed the namespace data") class Config: - # pydantic v1 - orm_mode = True - # pydantic v2 - from_attributes = True + if pydantic_version >= "2": + from_attributes = True + else: + orm_mode = True diff --git a/horizon/commons/schemas/v1/pagination.py b/horizon/commons/schemas/v1/pagination.py index e485b5a9..10f86e55 100644 --- a/horizon/commons/schemas/v1/pagination.py +++ b/horizon/commons/schemas/v1/pagination.py @@ -3,7 +3,12 @@ from typing import Generic, List, Optional, TypeVar from pydantic import BaseModel, Field -from pydantic.generics import GenericModel +from pydantic import __version__ as pydantic_version + +if pydantic_version >= "2": + from pydantic import BaseModel as GenericModel +else: + from pydantic.generics import GenericModel # type: ignore[no-redef] # noqa: WPS440 from horizon.commons.dto import Pagination diff --git a/horizon/commons/schemas/v1/permission.py b/horizon/commons/schemas/v1/permission.py index 9e0a9f85..839b2af1 100644 --- a/horizon/commons/schemas/v1/permission.py +++ b/horizon/commons/schemas/v1/permission.py @@ -40,7 +40,10 @@ class PermissionsUpdateRequestV1(BaseModel): ) @validator("permissions") - def _ensure_unique_usernames_and_single_owner(cls, permissions): + def _ensure_unique_usernames_and_single_owner( + cls, + permissions: List[PermissionUpdateRequestItemV1], + ) -> List[PermissionUpdateRequestItemV1]: seen: Set[str] = set() owner_count = 0 for perm in permissions: diff --git a/horizon/commons/schemas/v1/user.py b/horizon/commons/schemas/v1/user.py index 12a77a56..34db0b47 100644 --- a/horizon/commons/schemas/v1/user.py +++ b/horizon/commons/schemas/v1/user.py @@ -3,6 +3,7 @@ from __future__ import annotations from pydantic import BaseModel, Field +from pydantic import __version__ as pydantic_version class UserResponseV1(BaseModel): @@ -12,10 +13,10 @@ class UserResponseV1(BaseModel): username: str = Field(description="User name, unique in the entire database") class Config: - # pydantic v1 - orm_mode = True - # pydantic v2 - from_attributes = True + if pydantic_version >= "2": + from_attributes = True + else: + orm_mode = True class UserResponseV1WithAdmin(UserResponseV1): diff --git a/pyproject.toml b/pyproject.toml index dad4256f..2bbd94b6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -280,6 +280,7 @@ known_first_party = ["horizon", "tests"] [tool.mypy] python_version = "3.8" plugins = ["pydantic.mypy", "sqlalchemy.ext.mypy.plugin"] +strict_optional = true [[tool.mypy.overrides]] module = "sqlalchemy_utils" diff --git a/tests/test_backend/test_hwm/test_copy_hmws.py b/tests/test_backend/test_hwm/test_copy_hmws.py index 62e66622..cb54b35f 100644 --- a/tests/test_backend/test_hwm/test_copy_hmws.py +++ b/tests/test_backend/test_hwm/test_copy_hmws.py @@ -5,9 +5,10 @@ from typing import TYPE_CHECKING import pytest +from pydantic import __version__ as pydantic_version from sqlalchemy import select -from horizon.backend.db.models import HWM, Namespace, NamespaceUserRoleInt, User +from horizon.backend.db.models import HWM, Namespace, NamespaceUserRoleInt from horizon.backend.db.models.hwm_history import HWMHistory if TYPE_CHECKING: @@ -49,19 +50,46 @@ async def test_copy_hwms_same_source_and_target_namespace( namespace: Namespace, hwms: list[HWM], ): + body = { + "source_namespace_id": namespace.id, + "target_namespace_id": namespace.id, + "hwm_ids": [hwm.id for hwm in hwms], + "with_history": False, + } + response = await test_client.post( "v1/hwm/copy", headers={"Authorization": f"Bearer {access_token}"}, - json={ - "source_namespace_id": namespace.id, - "target_namespace_id": namespace.id, - "hwm_ids": [hwm.id for hwm in hwms], - "with_history": False, - }, + json=body, ) assert response.status_code == 422 - assert "Source and target namespace IDs must not be the same" in response.text + + if pydantic_version >= "2": + detail = { + "code": "value_error", + "context": {}, + "input": body, + "location": ["body"], + "message": "Value error, Source and target namespace IDs must not be the same.", + "url": "https://errors.pydantic.dev/2.5/v/value_error", + } + else: + detail = { + "code": "value_error", + "location": ["body", "__root__"], + "message": "Source and target namespace IDs must not be the same.", + } + + expected_body = { + "error": { + "code": "invalid_request", + "details": [detail], + "message": "Invalid request", + }, + } + + assert response.json() == expected_body @pytest.mark.asyncio @@ -149,7 +177,32 @@ async def test_copy_hwms_empty_hwm_ids_list( ) assert response.status_code == 422 - assert "List should have at least 1 item after validation, not 0" in response.text + + if pydantic_version >= "2": + detail = { + "code": "value_error", + "context": {}, + "input": [], + "location": ["body", "hwm_ids"], + "message": "Value error, List should have at least 1 item after validation, not 0", + "url": "https://errors.pydantic.dev/2.5/v/value_error", + } + else: + detail = { + "code": "value_error", + "location": ["body", "hwm_ids"], + "message": "List should have at least 1 item after validation, not 0", + } + + expected_body = { + "error": { + "code": "invalid_request", + "details": [detail], + "message": "Invalid request", + }, + } + + assert response.json() == expected_body @pytest.mark.asyncio @@ -267,7 +320,7 @@ async def test_copy_hwms_with_existing_hwm_name( "code": "already_exists", "details": {"entity_type": "HWM", "field": "name", "value": hwms[0].name}, "message": f"HWM with name={hwms[0].name!r} already exists", - } + }, } @@ -281,12 +334,12 @@ async def test_copy_hwms_with_existing_hwm_name( { "error": { "code": "permission_denied", - "message": f"Permission denied. User has role GUEST but action requires at least DEVELOPER.", + "message": "Permission denied. User has role GUEST but action requires at least DEVELOPER.", "details": { "required_role": "DEVELOPER", "actual_role": "GUEST", }, - } + }, }, ), ], diff --git a/tests/test_backend/test_hwm/test_create_hwm.py b/tests/test_backend/test_hwm/test_create_hwm.py index 6261d975..2a1bc862 100644 --- a/tests/test_backend/test_hwm/test_create_hwm.py +++ b/tests/test_backend/test_hwm/test_create_hwm.py @@ -557,7 +557,7 @@ async def test_create_hwm_with_same_name_after_deletion( "required_role": "DEVELOPER", "actual_role": "GUEST", }, - } + }, }, ), ], diff --git a/tests/test_backend/test_permissions/test_update_permissions.py b/tests/test_backend/test_permissions/test_update_permissions.py index e9bf9a59..297f0200 100644 --- a/tests/test_backend/test_permissions/test_update_permissions.py +++ b/tests/test_backend/test_permissions/test_update_permissions.py @@ -6,7 +6,7 @@ from typing import TYPE_CHECKING import pytest -from pydantic import VERSION +from pydantic import __version__ as pydantic_version from horizon.backend.db.models import Namespace, NamespaceUserRoleInt, User @@ -16,15 +16,6 @@ pytestmark = [pytest.mark.backend, pytest.mark.asyncio] -@pytest.fixture -def pydantic_url_value_error(): - pydantic_version = VERSION - if pydantic_version.startswith("1"): - return "https://errors.pydantic.dev/1.x/v/value_error" - else: - return "https://errors.pydantic.dev/2.5/v/value_error" - - async def test_update_namespace_permissions_unauthorized_user( test_client: AsyncClient, namespace: Namespace, @@ -93,7 +84,6 @@ async def test_update_namespace_permissions_with_duplicates_usernames( access_token: str, namespace_with_users: None, namespace: Namespace, - pydantic_url_value_error: str, ): changes = { "permissions": [ @@ -108,27 +98,36 @@ async def test_update_namespace_permissions_with_duplicates_usernames( json=changes, ) assert response.status_code == 422 - expected_content = { + + if pydantic_version >= "2": + detail = { + "code": "value_error", + "context": {}, + "input": [ + {"role": "DEVELOPER", "username": "user1"}, + {"role": "MAINTAINER", "username": "user1"}, + {"role": "OWNER", "username": "user2"}, + ], + "location": ["body", "permissions"], + "message": "Value error, Duplicate username detected: user1. Each username must appear only once.", + "url": "https://errors.pydantic.dev/2.5/v/value_error", + } + else: + detail = { + "code": "value_error", + "location": ["body", "permissions"], + "message": "Duplicate username detected: user1. Each username must appear only once.", + } + + expected_body = { "error": { "code": "invalid_request", - "details": [ - { - "code": "value_error", - "context": {}, - "input": [ - {"role": "DEVELOPER", "username": "user1"}, - {"role": "MAINTAINER", "username": "user1"}, - {"role": "OWNER", "username": "user2"}, - ], - "location": ["body", "permissions"], - "message": "Value error, Duplicate username detected: user1. Each username must appear only once.", - "url": pydantic_url_value_error, - } - ], + "details": [detail], "message": "Invalid request", - } + }, } - assert response.json() == expected_content + + assert response.json() == expected_body @pytest.mark.parametrize( @@ -147,7 +146,6 @@ async def test_update_namespace_permissions_duplicates_owner( access_token: str, namespace_with_users: None, namespace: Namespace, - pydantic_url_value_error: str, ): changes = { "permissions": [ @@ -162,24 +160,32 @@ async def test_update_namespace_permissions_duplicates_owner( json=changes, ) assert response.status_code == 422 - expected_response = { + + if pydantic_version >= "2": + detail = { + "code": "value_error", + "context": {}, + "input": changes["permissions"], + "location": ["body", "permissions"], + "message": "Value error, Multiple owner role assignments detected. Only one owner can be assigned.", + "url": "https://errors.pydantic.dev/2.5/v/value_error", + } + else: + detail = { + "code": "value_error", + "location": ["body", "permissions"], + "message": "Multiple owner role assignments detected. Only one owner can be assigned.", + } + + expected_body = { "error": { "code": "invalid_request", - "details": [ - { - "code": "value_error", - "context": {}, - "input": changes["permissions"], - "location": ["body", "permissions"], - "message": "Value error, Multiple owner role assignments detected. Only one owner can be assigned.", - "url": pydantic_url_value_error, - } - ], + "details": [detail], "message": "Invalid request", - } + }, } - assert response.json() == expected_response + assert response.json() == expected_body @pytest.mark.parametrize(