Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for federation in schema codegen #3417

Merged
merged 31 commits into from
Mar 29, 2024

Conversation

patrick91
Copy link
Member

Pretty much the title, it could be made more solid, but I think
it is pretty good for most use cases

@patrick91
Copy link
Member Author

/pre-release

@botberry
Copy link
Member

botberry commented Mar 20, 2024

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


This release adds support for Apollo Federation in the schema codegen. Now you
can convert a schema like this:

extend schema
  @link(url: "https://specs.apollo.dev/federation/v2.3",
        import: ["@key", "@shareable"])

type Query {
  me: User
}

type User @key(fields: "id") {
  id: ID!
  username: String! @shareable
}

to a Strawberry powered schema like this:

import strawberry


@strawberry.type
class Query:
    me: User | None


@strawberry.federation.type(keys=["id"])
class User:
    id: strawberry.ID
    username: str = strawberry.federation.field(shareable=True)


schema = strawberry.federation.Schema(query=Query, enable_federation_2=True)

By running the following command:

strawberry schema-codegen example.graphql

Here's the tweet text:

🆕 Release (next) is out! Thanks to @patrick91 for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

@botberry
Copy link
Member

Pre-release

👋

Pre-release 0.221.0.dev.1710955937 [dbe6b25] has been released on PyPi! 🚀
You can try it by doing:

poetry add strawberry-graphql==0.221.0.dev.1710955937

Copy link

codecov bot commented Mar 20, 2024

Codecov Report

Merging #3417 (50e8ee8) into main (22838db) will increase coverage by 0.01%.
Report is 7 commits behind head on main.
The diff coverage is 92.02%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3417      +/-   ##
==========================================
+ Coverage   96.40%   96.41%   +0.01%     
==========================================
  Files         498      507       +9     
  Lines       31137    32178    +1041     
  Branches     3815     5325    +1510     
==========================================
+ Hits        30018    31026    +1008     
- Misses        913      941      +28     
- Partials      206      211       +5     

@strawberry-graphql strawberry-graphql deleted a comment from botberry Mar 20, 2024
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @patrick91 - I've reviewed your changes and they look great!

General suggestions:

  • Consider extending the _sanitize_argument function to handle additional data types beyond strings and booleans for future-proofing.
  • Ensure that the code correctly handles non-federation fields and provides a way to opt-out of federation features explicitly if needed.
  • Validate the assumption that federation schema extensions are always declared at the top of the document.
  • Expand testing to cover negative scenarios, mixed federation and non-federation schemas, recursive types, and the application of multiple federation directives on the same type or field.
Here's what I looked at during the review
  • 🟡 General issues: 4 issues found
  • 🟢 Security: all looks good
  • 🟡 Testing: 5 issues found
  • 🟢 Complexity: all looks good
  • 🟢 Docstrings: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

strawberry/schema_codegen/__init__.py Outdated Show resolved Hide resolved
strawberry/schema_codegen/__init__.py Outdated Show resolved Hide resolved
strawberry/schema_codegen/__init__.py Show resolved Hide resolved
strawberry/schema_codegen/__init__.py Show resolved Hide resolved
@@ -0,0 +1,157 @@
import textwrap
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Consider adding negative test cases for federation features.

While the provided tests cover the positive scenarios well, adding negative test cases (e.g., when directives are malformed or missing expected arguments) would ensure robust error handling and validation.

tests/schema_codegen/test_federation.py Show resolved Hide resolved
tests/schema_codegen/test_federation.py Show resolved Hide resolved
tests/schema_codegen/test_federation.py Show resolved Hide resolved
tests/schema_codegen/test_federation.py Show resolved Hide resolved
strawberry/schema_codegen/__init__.py Outdated Show resolved Hide resolved
Copy link

codspeed-hq bot commented Mar 20, 2024

CodSpeed Performance Report

Merging #3417 will not alter performance

Comparing feature/federation-schema-codegen (50e8ee8) with main (9ef6690)

Summary

✅ 13 untouched benchmarks

@botberry
Copy link
Member

botberry commented Mar 28, 2024

Apollo Federation Subgraph Compatibility Results

Federation 1 Support Federation 2 Support
_service🟢
@key (single)🟢
@key (multi)🟢
@key (composite)🟢
repeatable @key🟢
@requires🟢
@provides🟢
federated tracing🔲
@link🟢
@shareable🟢
@tag🟢
@override🟢
@inaccessible🟢
@composeDirective🟢
@interfaceObject🟢

Learn more:

@patrick91 patrick91 force-pushed the feature/federation-schema-codegen branch from 2bdb135 to 8ebbe25 Compare March 29, 2024 09:51
@patrick91 patrick91 force-pushed the feature/federation-schema-codegen branch from cf9e13b to 7d50896 Compare March 29, 2024 10:29
@patrick91 patrick91 force-pushed the feature/federation-schema-codegen branch from d7c4554 to df461db Compare March 29, 2024 14:13
@patrick91 patrick91 merged commit 848d00b into main Mar 29, 2024
63 of 64 checks passed
@patrick91 patrick91 deleted the feature/federation-schema-codegen branch March 29, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants