From 92cc106d8a85084920b6cfdfd2f7bd2f07099f5d Mon Sep 17 00:00:00 2001 From: tjeerddie Date: Thu, 7 Nov 2024 16:15:24 +0100 Subject: [PATCH] Upgrade strawberry graphql to 0.246.2 and above (#777) * Upgrade strawberry graphql to 0.236.0 and above * Upgrade strawberry to above 0.246.2 and upgrade nwa-stdlib * Change strawberry graphql to earliest failing update for debugging - revert to old imports - add temp log for models * remove blocks from strawberry_models * Fix duplicate type error in strawberry.federation.schema The problem was that the `autoregistration` unnecessarily created a type based on the output of the `pydantic_wrapper` function, which already generates the required Strawberry type. This led to the same Strawberry type being created twice at runtime, but from different locations: 1. Directly in the `pydantic_wrapper`, resulting in `strawberry.experimental.pydantic.object_type.YourStrawberryType`. 2. Indirectly in `autoregistration`, which created a type using the result of `pydantic_wrapper`, resulting in `orchestrator.graphql.autoregistration.YourStrawberryType` - with the function `create_block_strawberry_type` or `create_subscription_strawberry_type`. Previously Strawberry handled this duplication because it already needed to convert Strawberry types to GraphQL types, so we never noticed that we created the type twice. Strawberry refactored `strawberry.federation.schema` to use Strawberry types directly (removing GraphQL type conversion) in version `0.233.0`. This implementation does not unintentionally fix our duplicate types and resulted in the runtime error: `Union type _Entity can only include type YourStrawberryType once`. Removing the type create in `autoregistration` fixed this problem. * Change strawberry-graphql back to 0.246.2 or above * Add sort_by to test_product_blocks_has_previous_page * Bumpversion to 2.9.0rc1 --- .bumpversion.cfg | 2 +- orchestrator/__init__.py | 2 +- orchestrator/graphql/autoregistration.py | 9 +++++---- orchestrator/graphql/schemas/process.py | 2 +- orchestrator/graphql/schemas/product.py | 2 +- .../graphql/schemas/strawberry_pydantic_patch.py | 2 +- orchestrator/graphql/schemas/subscription.py | 2 +- orchestrator/graphql/types.py | 2 +- orchestrator/graphql/utils/override_class.py | 2 +- pyproject.toml | 4 ++-- test/unit_tests/graphql/conftest.py | 3 --- test/unit_tests/graphql/test_product_blocks.py | 2 +- 12 files changed, 16 insertions(+), 18 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index ce7d8f5b3..36b8a5cef 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 2.8.0 +current_version = 2.9.0rc1 commit = False tag = False parse = (?P\d+)\.(?P\d+)\.(?P\d+)(rc(?P\d+))? diff --git a/orchestrator/__init__.py b/orchestrator/__init__.py index 8d047a217..564dda1d0 100644 --- a/orchestrator/__init__.py +++ b/orchestrator/__init__.py @@ -13,7 +13,7 @@ """This is the orchestrator workflow engine.""" -__version__ = "2.8.0" +__version__ = "2.9.0rc1" from orchestrator.app import OrchestratorCore from orchestrator.settings import app_settings diff --git a/orchestrator/graphql/autoregistration.py b/orchestrator/graphql/autoregistration.py index 1d55010fb..1719cc3d1 100644 --- a/orchestrator/graphql/autoregistration.py +++ b/orchestrator/graphql/autoregistration.py @@ -17,9 +17,9 @@ import strawberry import structlog from more_itertools import one +from strawberry import UNSET from strawberry.experimental.pydantic.conversion_types import StrawberryTypeFromPydantic from strawberry.federation.schema_directives import Key -from strawberry.unset import UNSET from orchestrator.domain import SUBSCRIPTION_MODEL_REGISTRY from orchestrator.domain.base import DomainModel, get_depends_on_product_block_type_list @@ -76,8 +76,8 @@ def create_block_strawberry_type( strawberry_name: str, model: type[DomainModel], ) -> type[StrawberryTypeFromPydantic[DomainModel]]: + from strawberry import UNSET from strawberry.federation.schema_directives import Key - from strawberry.unset import UNSET federation_key_directives = [Key(fields="subscriptionInstanceId", resolvable=UNSET)] @@ -92,7 +92,7 @@ def create_block_strawberry_type( description=f"{strawberry_name} Type", use_pydantic_alias=USE_PYDANTIC_ALIAS_MODEL_MAPPING.get(strawberry_name, True), ) - return type(strawberry_name, (pydantic_wrapper(base_type),), {}) + return pydantic_wrapper(base_type) def create_subscription_strawberry_type(strawberry_name: str, model: type[DomainModel], interface: type) -> type: @@ -106,7 +106,7 @@ def create_subscription_strawberry_type(strawberry_name: str, model: type[Domain description=f"{strawberry_name} Type", use_pydantic_alias=USE_PYDANTIC_ALIAS_MODEL_MAPPING.get(strawberry_name, True), ) - return type(strawberry_name, (pydantic_wrapper(base_type),), {}) + return pydantic_wrapper(base_type) def add_class_to_strawberry( @@ -146,6 +146,7 @@ def register_domain_models( for product_type in SUBSCRIPTION_MODEL_REGISTRY.values() if product_type.__base_type__ } + for key, product_type in products.items(): add_class_to_strawberry( interface=interface, diff --git a/orchestrator/graphql/schemas/process.py b/orchestrator/graphql/schemas/process.py index d9208554f..4e5d12d1b 100644 --- a/orchestrator/graphql/schemas/process.py +++ b/orchestrator/graphql/schemas/process.py @@ -1,9 +1,9 @@ from typing import TYPE_CHECKING, Annotated import strawberry +from strawberry import UNSET from strawberry.federation.schema_directives import Key from strawberry.scalars import JSON -from strawberry.unset import UNSET from oauth2_lib.strawberry import authenticated_field from orchestrator.db import ProcessTable, ProductTable, db diff --git a/orchestrator/graphql/schemas/product.py b/orchestrator/graphql/schemas/product.py index 577226c65..6679b5e59 100644 --- a/orchestrator/graphql/schemas/product.py +++ b/orchestrator/graphql/schemas/product.py @@ -1,8 +1,8 @@ from typing import TYPE_CHECKING, Annotated import strawberry +from strawberry import UNSET from strawberry.federation.schema_directives import Key -from strawberry.unset import UNSET from oauth2_lib.strawberry import authenticated_field from orchestrator.db import ProductTable diff --git a/orchestrator/graphql/schemas/strawberry_pydantic_patch.py b/orchestrator/graphql/schemas/strawberry_pydantic_patch.py index fc7129ff0..051c9c5bf 100644 --- a/orchestrator/graphql/schemas/strawberry_pydantic_patch.py +++ b/orchestrator/graphql/schemas/strawberry_pydantic_patch.py @@ -2,7 +2,7 @@ import strawberry from strawberry.experimental.pydantic.conversion import _convert_from_pydantic_to_strawberry_type -from strawberry.field import StrawberryField +from strawberry.types.field import StrawberryField # Vendored convert_pydantic_model_to_strawberry_class from # https://github.com/strawberry-graphql/strawberry/blob/d721eb33176cfe22be5e47f5bf2c21a4a022a6d6/strawberry/experimental/pydantic/conversion.py diff --git a/orchestrator/graphql/schemas/subscription.py b/orchestrator/graphql/schemas/subscription.py index ea0bbad01..efe61c9e3 100644 --- a/orchestrator/graphql/schemas/subscription.py +++ b/orchestrator/graphql/schemas/subscription.py @@ -5,8 +5,8 @@ import strawberry from pydantic import BaseModel from sqlalchemy import select +from strawberry import UNSET from strawberry.federation.schema_directives import Key -from strawberry.unset import UNSET from oauth2_lib.strawberry import authenticated_field from orchestrator.db import FixedInputTable, ProductTable, SubscriptionTable, db diff --git a/orchestrator/graphql/types.py b/orchestrator/graphql/types.py index 0bb07dbdf..7fd781873 100644 --- a/orchestrator/graphql/types.py +++ b/orchestrator/graphql/types.py @@ -18,12 +18,12 @@ import strawberry from graphql import GraphQLError -from strawberry.custom_scalar import ScalarDefinition, ScalarWrapper from strawberry.dataloader import DataLoader from strawberry.experimental.pydantic.conversion_types import StrawberryTypeFromPydantic from strawberry.scalars import JSON from strawberry.types import Info from strawberry.types.info import RootValueType +from strawberry.types.scalar import ScalarDefinition, ScalarWrapper from nwastdlib.vlans import VlanRanges from oauth2_lib.fastapi import AuthManager diff --git a/orchestrator/graphql/utils/override_class.py b/orchestrator/graphql/utils/override_class.py index dc32b5809..e065e0161 100644 --- a/orchestrator/graphql/utils/override_class.py +++ b/orchestrator/graphql/utils/override_class.py @@ -1,4 +1,4 @@ -from strawberry.field import StrawberryField +from strawberry.types.field import StrawberryField def override_class(strawberry_class: type, fields: list[StrawberryField]) -> type: diff --git a/pyproject.toml b/pyproject.toml index eac69fb34..47be1b047 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,10 +60,10 @@ dependencies = [ "structlog", "typer==0.12.5", "uvicorn[standard]~=0.32.0", - "nwa-stdlib~=1.7.3", + "nwa-stdlib~=1.8.0", "oauth2-lib~=2.3.0", "tabulate==0.9.0", - "strawberry-graphql==0.232.2", + "strawberry-graphql>=0.246.2", "pydantic-forms==1.1.0", ] diff --git a/test/unit_tests/graphql/conftest.py b/test/unit_tests/graphql/conftest.py index 0809cfa0e..0b22a0a36 100644 --- a/test/unit_tests/graphql/conftest.py +++ b/test/unit_tests/graphql/conftest.py @@ -41,9 +41,6 @@ class Metadata(BaseModel): @pytest.fixture(scope="session", autouse=True) def fix_graphql_model_registration(): - # This block caches the "ProductBlockListNestedForTestInactive" model to avoid re-instantiation in each test case. - # This is necessary because this product block has a self referencing property, which strawberry can't handle correctly, - # and lead to an error expecting the `ProductBlockListNestedForTestInactive` strawberry type to already exist. # This block caches the "ProductBlockListNestedForTestInactive" model to avoid re-instantiation in each test case. # This is necessary because this product block has a self referencing property, which strawberry can't handle correctly, # and lead to an error expecting the `ProductBlockListNestedForTestInactive` strawberry type to already exist. diff --git a/test/unit_tests/graphql/test_product_blocks.py b/test/unit_tests/graphql/test_product_blocks.py index edeac007f..7199d4690 100644 --- a/test/unit_tests/graphql/test_product_blocks.py +++ b/test/unit_tests/graphql/test_product_blocks.py @@ -220,7 +220,7 @@ def test_product_block_query_with_relations(test_client, query_args): def test_product_blocks_has_previous_page(test_client): - data = get_product_blocks_query(after=1) + data = get_product_blocks_query(after=1, sort_by=[{"field": "name", "order": "ASC"}]) response: Response = test_client.post("/api/graphql", content=data, headers={"Content-Type": "application/json"}) assert HTTPStatus.OK == response.status_code