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

Run strawberry codegen on top of graphql schema. #3221

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mgilson
Copy link
Contributor

@mgilson mgilson commented Nov 13, 2023

There aren't many graphql client generators in the python ecosystem. AFAIK, strawberry and https://github.com/mirumee/ariadne-codegen are the only two. For organizations already using strawberry on the backend, it makes a lot of sense to continue to use strawberry codegen, however it would be nice to be able to use the same custom plugins for strawberry services and other services that aren't using strawberry on the backend (e.g. github).

From the strawberry codegen perspective, this really just means swapping out the schema object with a separate object with a compatible interface. Luckily, there schema object has a very small used-surface area in the query-codegen code so making this swap isn't terribly difficult.

Description

Enable building client objects on top of graphql schema files. Future changes could allow this to happen on top of introspection URLs.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@botberry
Copy link
Member

botberry commented Nov 13, 2023

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


Allow strawberry's code generator to build clients from graphql schema files.

Here's the tweet text:

🆕 Release (next) is out! Thanks to Matt Gilson for the PR 👏

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

Copy link

codspeed-hq bot commented Nov 13, 2023

CodSpeed Performance Report

Merging #3221 will not alter performance

Comparing mgilson:codegen-from-schema (3710960) with main (4130a75)

Summary

✅ 11 untouched benchmarks

Copy link

codecov bot commented Nov 13, 2023

Codecov Report

Merging #3221 (3710960) into main (4130a75) will decrease coverage by 0.12%.
The diff coverage is 73.58%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3221      +/-   ##
==========================================
- Coverage   96.60%   96.48%   -0.12%     
==========================================
  Files         481      482       +1     
  Lines       29928    30083     +155     
  Branches     3691     3719      +28     
==========================================
+ Hits        28911    29026     +115     
- Misses        833      858      +25     
- Partials      184      199      +15     

@mgilson mgilson force-pushed the codegen-from-schema branch from 3b75011 to 09b06ce Compare November 13, 2023 15:28
Copy link
Member

@patrick91 patrick91 left a comment

Choose a reason for hiding this comment

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

This looks great, I left a couple of minor things, I think it might be worth document this though

@@ -0,0 +1,3 @@
Release type: minor

Allow strawberry's code generator to build clients from graphql schema files.
Copy link
Member

Choose a reason for hiding this comment

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

this is an awesome feature, we should extend this release note :)

Comment on lines +129 to +131
if schema.endswith(".graphql"):
with Path(schema).open() as input_schema:
schema_symbol = GraphQLSchemaWrapper(build_schema(input_schema.read()))
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should use a different flag for this :)

Also I prefer to remove one nesting level using read_text

Suggested change
if schema.endswith(".graphql"):
with Path(schema).open() as input_schema:
schema_symbol = GraphQLSchemaWrapper(build_schema(input_schema.read()))
if schema.endswith(".graphql"):
schema = Path(schema).read_text()
schema_symbol = GraphQLSchemaWrapper(build_schema(schema))

skip_snapshot_test = False
if query.name == "custom_scalar.graphql":
# The default plugins don't support custom types built this way
# (because it doesn't know the python on Typscript types).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# (because it doesn't know the python on Typscript types).
# (because it doesn't know the python or typescript types).


class SchemaConverterLike(Protocol):
@property
def scalar_registry(self) -> Registry:
Copy link
Member

Choose a reason for hiding this comment

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

can we add some pragma no cover here? 😊

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.

3 participants