Skip to content

Commit

Permalink
Upgrade strawberry graphql to 0.246.2 and above (#777)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
tjeerddie authored Nov 7, 2024
1 parent 23edbff commit 92cc106
Show file tree
Hide file tree
Showing 12 changed files with 16 additions and 18 deletions.
2 changes: 1 addition & 1 deletion .bumpversion.cfg
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[bumpversion]
current_version = 2.8.0
current_version = 2.9.0rc1
commit = False
tag = False
parse = (?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)(rc(?P<build>\d+))?
Expand Down
2 changes: 1 addition & 1 deletion orchestrator/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions orchestrator/graphql/autoregistration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)]

Expand All @@ -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:
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion orchestrator/graphql/schemas/process.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion orchestrator/graphql/schemas/product.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion orchestrator/graphql/schemas/strawberry_pydantic_patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion orchestrator/graphql/schemas/subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion orchestrator/graphql/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion orchestrator/graphql/utils/override_class.py
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]

Expand Down
3 changes: 0 additions & 3 deletions test/unit_tests/graphql/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion test/unit_tests/graphql/test_product_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 92cc106

Please sign in to comment.