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

added ability to override federation version on export schema command #3697

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

Conversation

iotflowadmin
Copy link

@iotflowadmin iotflowadmin commented Nov 14, 2024

Description

Added option for the export_schema command to override the output federation version.
ussage example: strawberry export-schema --federation-version=2.5

This change ONLY effect "strawberry export-schema" command and is fully backword-compatible without any logic change.

This enhancement is required to be able to work with routers (like: wundergraph cosmo - which currently support federation version only up to 2.5, or to use in organizations that can't currently update the federation version support of the router)

Warning: Please use with caution!!

If the schema define directives that are not supported by the specified version (in the override parameter) the schema
will still generate the output useing the value from the override, and may break at runtime.

Types of Changes

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

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

Copy link
Contributor

sourcery-ai bot commented Nov 14, 2024

Reviewer's Guide by Sourcery

This PR adds functionality to override the federation version when exporting a GraphQL schema using the export-schema command. The implementation modifies the schema directive URL during export and includes new test cases to verify the functionality.

Sequence diagram for export-schema command with federation version override

sequenceDiagram
    actor User
    participant CLI as CLI
    participant App as Application
    participant Schema as Schema

    User->>CLI: Run export-schema with --federation-version
    CLI->>App: Set federation_version_override
    App->>Schema: Modify schema directive URL
    Schema-->>App: Return modified schema
    App-->>CLI: Output schema
    CLI-->>User: Display schema with overridden federation version
Loading

Updated class diagram for ImportedFrom class

classDiagram
    class ImportedFrom {
        +String name
        +String url
        +__init__(**kwargs: Dict[str, Any])
    }
    note for ImportedFrom "Added logic to override URL based on federation_version_override"
Loading

File-Level Changes

Change Details Files
Added federation version override functionality to the export-schema command
  • Added new --federation-version command line option
  • Implemented version override logic in ImportedFrom class
  • Added federation version base URL constant
  • Added warning about version compatibility risks
strawberry/cli/commands/export_schema.py
strawberry/federation/schema_directives.py
Added test coverage for federation version override feature
  • Created new test case for federation version override
  • Added sample federated module for testing
  • Verified schema output with custom federation version
tests/cli/test_export_schema.py
tests/fixtures/sample_package/sample_module_federated.py
Updated documentation with new feature details
  • Added usage example for federation version override
  • Included warning about version compatibility
  • Added explanation of potential runtime issues
docs/guides/schema-export.md
RELEASE.md

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

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 @iotflowadmin - I've reviewed your changes - here's some feedback:

Overall Comments:

  • There are several typos in the documentation and help text that should be fixed: 'backword' -> 'backward', 'useing' -> 'using', 'brake' -> 'break', 'ussage' -> 'usage', 'effect' -> 'affect'
Here's what I looked at during the review
  • 🟡 General issues: 5 issues found
  • 🟢 Security: all looks good
  • 🟡 Testing: 1 issue found
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: 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 and I'll use the feedback to improve your reviews.

strawberry/cli/commands/export_schema.py Outdated Show resolved Hide resolved
tests/cli/test_export_schema.py Show resolved Hide resolved
docs/guides/schema-export.md Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
@botberry
Copy link
Member

botberry commented Nov 14, 2024

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


Added option for the export_schema command to override the output federation version.
usage example: strawberry export-schema --federation-version=2.5

This change ONLY affects "strawberry export-schema" command and is fully backward-compatible without any logic change.

Warning: Please use with caution!!

If the schema define directives that are not supported by the specified version (in the override parameter) the schema
will still generate the output using the value from the override, and may break at runtime.

Here's the tweet text:

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

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

# added negative and backward-compatibility tests
@patrick91
Copy link
Member

@iotflowadmin thanks for the PR, I was wondering if we could specify the federation version when creating the schema instead, that way we could also disable some newer directives, what do you think?

@iotflowadmin
Copy link
Author

iotflowadmin commented Nov 22, 2024

@iotflowadmin thanks for the PR, I was wondering if we could specify the federation version when creating the schema instead, that way we could also disable some newer directives, what do you think?

@patrick91 - I think you are right, that would be a better approach. (it will enable to add some sort of IDE plugin at least for the common once, to validate on the fly that the select version actually support all the directives - should not be too hard with static code inspection).

but after digging a bit deeper in the code, I sow that it will include more effort (addressing it in more components that take place in runtime, and not only in generation.) and I just needed a quick fix cause I was testing migration from the guild which support up to 2.7 to wundergraph which currently support up to 2.5 (and found when looking around several people with same issue most of them just eventually changed it manually).

I think its still worth while adding it to the schema creation and may be I will send another PR when I find a bit more time to fiddle with it, but this tow approaches do not necessarily cancel each other and can serve for different purposes
for example: I want to deploy the schema to tow environments with different max version support and I want to be on latest available on both... so I would do it with the schema generation as part of my CI/CD process, rather then managing separate code version for each environment.

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