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 supported_schemas #2609

Closed
wants to merge 2 commits into from

Conversation

vdurmont
Copy link

@vdurmont vdurmont commented Mar 5, 2023

Motivations

Running multiple version of the API

While GraphQL APIs are usually not versioned and should strive to never introduce
breaking changes, the fact is that you may want to version your GraphQL API and
run multiple versions in parallel within the same server.

Instead of duplicating code or creating a complex class hierarchy, supported_schemas
enable you to annotate a field and specify in which version this field should be
available.

Running multiple schemas using "similar" objects

Sometimes, you may want to serve multiple GraphQL APIs that use the same objects
but with slightly different fields. A good example is that your third party GraphQL
API might expose the "core" fields of an object but your first party API might
add more information and more fields for your first party apps.

Instead of duplicating code or creating a complex class hierarchy, supported_schemas
enable you to annotate a field and specify in which schemas those fields should
be available.

How it works

Adds an optional supported_schemas feature to the fields.

One can decide to limit the support for a particular field to:

  • A schema designated by name,
  • A schema, until a specific version,
  • A schema, from a specific version.

When the GraphQL schema is being built, we may pass an identifier for the current schema name and version and the fields will be filtered appropriately.

Examples of field definitions:

@strawberry.type
class User:
    # [...]

    @strawberry.field(name="debugMessage", supported_schemas=[
        SupportedSchema(name="internal"),
    ])
    def get_debug_message(self) -> str:
        # This field will only appear in the schemas called `internal`
        # [...]

    @strawberry.field(name="riskScore", supported_schemas=[
        SupportedSchema(name="internal", until_version="1.2"),
    ])
    def get_old_risk_score(self) -> float:
        # This field will only appear in the schemas called `internal` that have
        # a version lower than or equal to `1.2`
        # [...]

    @strawberry.field(name="riskScore", supported_schemas=[
        SupportedSchema(name="internal", from_version="1.3"),
    ])
    def get_new_risk_score(self) -> float:
        # This field will only appear in the schemas called `internal` that have
        # a version higher than or equal to `1.3`
        # [...]

Examples of schema definition:

internal_schema = Schema(
    query=Query,
    mutation=Mutation,
    schema_identifier=SchemaIdentifier(name="internal", version="1.4"),
)

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).

I couldn't get the pre-commit hook to run but ran black manually:

$ poetry run pre-commit install
Command not found: pre-commit

@codecov
Copy link

codecov bot commented Mar 5, 2023

Codecov Report

Merging #2609 (77f100e) into main (dd5e388) will decrease coverage by 0.46%.
Report is 6 commits behind head on main.
The diff coverage is 14.81%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2609      +/-   ##
==========================================
- Coverage   96.48%   96.02%   -0.46%     
==========================================
  Files         467      471       +4     
  Lines       29166    29395     +229     
  Branches     3592     3623      +31     
==========================================
+ Hits        28140    28227      +87     
- Misses        843      975     +132     
- Partials      183      193      +10     

@botberry
Copy link
Member

botberry commented Mar 5, 2023

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


This release adds an optional supported_schemas feature to the fields.

One can decide to limit the support for a particular field to:

  • A schema designated by name,
  • A schema, until a specific version,
  • A schema, from a specific version.

When the GraphQL schema is being built, we may pass an identifier for the current
schema name and version and the fields will be filtered appropriately.

Examples of field definitions:

import strawberry
from strawberry.field import SupportedSchema

@strawberry.type
class User:
    # [...]

    @strawberry.field(name="debugMessage", supported_schemas=[
        SupportedSchema(name="internal"),
    ])
    def get_debug_message(self) -> str:
        # This field will only appear in the schemas called `internal`
        # [...]

    @strawberry.field(name="riskScore", supported_schemas=[
        SupportedSchema(name="internal", until_version="1.2"),
    ])
    def get_old_risk_score(self) -> float:
        # This field will only appear in the schemas called `internal` that have
        # a version lower than or equal to `1.2`
        # [...]

    @strawberry.field(name="riskScore", supported_schemas=[
        SupportedSchema(name="internal", from_version="1.3"),
    ])
    def get_new_risk_score(self) -> float:
        # This field will only appear in the schemas called `internal` that have
        # a version higher than or equal to `1.3`
        # [...]

Examples of schema definition:

import strawberry
from strawberry.schema.identifier import SchemaIdentifier

# [...] Define `Query` and `Mutation`

internal_schema = strawberry.Schema(
    query=Query,
    mutation=Mutation,
    schema_identifier=SchemaIdentifier(name="internal", version="1.4"),
)

Here's the preview release card for twitter:

Here's the tweet text:

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

Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)

@vdurmont
Copy link
Author

vdurmont commented Mar 5, 2023

Looks like I have some CI/CD failures. I'm happy to work on cleaning things up and fix them if the feature itself and the direction of the implementation make sense to the strawberry maintainers.

@jkimbo
Copy link
Member

jkimbo commented Mar 5, 2023

@vdurmont this is an interesting proposal but we'd rather avoid adding new parameters to the strawberry.field method if possible. Do you think this could be solved by using the proposed Object Type extensions: #2605 ?

(cc @erikwrede )

@erikwrede
Copy link
Member

erikwrede commented Mar 5, 2023

@jkimbo interesting suggestion. While an ObjectTypeExtension could help with the object type, it still needs some info about the fields that are affected. This is hard to achieve conveniently without additional info on each field. FieldExtensions in their current design are mainly intended to affect the field's behavior, so they aren't a good place to store field-specific info either. Using SchemaDirectives on the fields and types could be an alternative, as they are the perfect place to store metadata on a field:

@strawberry.type(directives=[SupportsSchemas(schemas=("internal","external")), SchemaVersion(lte="1.4")]))
class User:

    @strawberry.field(name="debugMessage", 
			directives=[SupportsSchemas(schemas=("internal",)), SchemaVersion(lte="1.4")])
	def debugField():
		pass

However, that doesn't solve the problem of building the schema. That schema-building would still need to involve custom logic to pass the current schema version to the type extension (unless we figure out an API for SchemaExtensions (currently Extension) to hook into the converter).

On the topic of multiple schemas using the same types, I do have some concerns about the maintenance and clarity of the codebase. While I understand the use case for schema versioning, I worry that allowing multiple named schemas (e.g. "internal", "external"...) with the same types and fields could encourage design of more complex and harder to maintain code. Instead, it might be better to provide customers adjusted SDLs for each API and ensure that permissions are handled properly in the background (field visibility vs actually different schemas) or looking into federating the schema. That being said, I do recognize that there are situations where this feature might be necessary or appropriate.

@jkimbo
Copy link
Member

jkimbo commented Mar 6, 2023

While an ObjectTypeExtension could help with the object type, it still needs some info about the fields that are affected.

@erikwrede you could use the metadata argument to fields to store this information.

That schema-building would still need to involve custom logic to pass the current schema version to the type extension (unless we figure out an API for SchemaExtensions (currently Extension) to hook into the converter).

Good point and IMO we should extend schema extensions to include hooks into the converter.

On the topic of multiple schemas using the same types, I do have some concerns about the maintenance and clarity of the codebase. While I understand the use case for schema versioning, I worry that allowing multiple named schemas (e.g. "internal", "external"...) with the same types and fields could encourage design of more complex and harder to maintain code.

I completely agree with this and it's something to be mindful of @vdurmont . I do think we should provide enough functionality through extensions so that people can experiment with functionality like this to see how it might work in practice.

@erikwrede
Copy link
Member

erikwrede commented Mar 6, 2023

@jkimbo Field Metadata is even better, I agree.
Maybe we should look at the two concerns this PR solves (schema versioning, schema variants) separately.

Revisiting the post in graphql-js I linked yesterday, it might be worthwhile to investigate visibility modifiers instead of the schema variants implementation of this PR. Inspired by graphql-ruby, a visibility modifier decides based on context wether a field is visible to a certain user in introspection and queries: https://graphql-ruby.org/authorization/visibility.html

Using these modifiers, internal and external schemas can be defined without building two separate schemas from the same types. This would reduce complexity by disabling the use of two fields with the same name that have different resolvers or return types. At the same time, we enable users to easily implement static and dynamic feature toggles, which is an important use case.

The problem with these modifiers might be a deep integration into GQL-Core (to ensure introspection & validation are behaving correctly). Additionally, we'd need to ensure that they are clearly differentiating these from Permissions so they aren't used interchangably.

In any way, if we come up with a design for SchemaExtensions that enables modifying the exposed fields, I'd favor that approach over integrating the feature right into the core schema converter.

@jkimbo
Copy link
Member

jkimbo commented Mar 6, 2023

@erikwrede yes I like the idea of adding visibility filters but it would require some customisation of graphql-core. More discussion here: #361

Adds an optional `supported_schemas` feature to the fields.

One can decide to limit the support for a particular field to:
- A schema designated by name,
- A schema, until a specific version,
- A schema, from a specific version.

When the GraphQL schema is being built, we may pass an identifier for the current
schema name and version and the fields will be filtered appropriately.

Examples of field definitions:

```python
@strawberry.type
class User:
    # [...]

    @strawberry.field(name="debugMessage", supported_schemas=[
        SupportedSchema(name="internal"),
    ])
    def get_debug_message(self) -> str:
        # This field will only appear in the schemas called `internal`
        # [...]

    @strawberry.field(name="riskScore", supported_schemas=[
        SupportedSchema(name="internal", until_version="1.2"),
    ])
    def get_old_risk_score(self) -> float:
        # This field will only appear in the schemas called `internal` that have
        # a version lower than or equal to `1.2`
        # [...]

    @strawberry.field(name="riskScore", supported_schemas=[
        SupportedSchema(name="internal", from_version="1.3"),
    ])
    def get_new_risk_score(self) -> float:
        # This field will only appear in the schemas called `internal` that have
        # a version higher than or equal to `1.3`
        # [...]
```

Examples of schema definition:

```python
internal_schema = strawberry.Schema(
    query=Query,
    mutation=Mutation,
    schema_identifier=SchemaIdentifier(name="internal", version="1.4"),
)
```
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 14, 2023

CodSpeed Performance Report

Merging #2609 will not alter performance

Comparing vdurmont:supported_schemas (77f100e) with main (1171e7f)

Summary

✅ 12 untouched benchmarks

@patrick91
Copy link
Member

@vdurmont sorry for keeping this PR in flight for so long, I totally forgot about it 😊

I've implement something similar using metadata here #3274, it's a much simpler approach with less opinions from us, so I'll close this issue, hope that's ok 😊

@patrick91 patrick91 closed this Jan 16, 2024
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.

5 participants