workfloworchestrator / orchestrator-core

The workflow orchestrator core repository
Apache License 2.0
45 stars 15 forks source link

Upgrade strawberry graphql to 0.246.2 and above #777

Closed tjeerddie closed 2 weeks ago

tjeerddie commented 2 weeks ago

upgrading to above 0.246.2 to be consistent with nwa-stdlib package. could also change it to above 0.236.0 to be consistent with the strawberry breaking import change.

I found the upgrading problem in strawberry version 0.233.0, where they refactor the federation schema to use strawberry typing instead of converting strawberry types to graphql types https://github.com/strawberry-graphql/strawberry/pull/3525

bumpversion to 2.9.0rc1 and strawberry breaking imports should be included in the release message.

Upgrade problem description

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.

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.76%. Comparing base (23edbff) to head (d22bbcb). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #777 +/- ## ======================================= Coverage 83.76% 83.76% ======================================= Files 190 190 Lines 9450 9450 Branches 924 924 ======================================= Hits 7916 7916 Misses 1283 1283 Partials 251 251 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.