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

GraphQL changes for 2.0 #3388

Merged
merged 14 commits into from
Jan 26, 2024
Merged

GraphQL changes for 2.0 #3388

merged 14 commits into from
Jan 26, 2024

Conversation

TonyCTHsu
Copy link
Contributor

@TonyCTHsu TonyCTHsu commented Jan 19, 2024

2.0 Upgrade Guide notes

🚨 Breaking changes:

  1. No longer support and patch GraphQL's defined-based schema

  2. Support graphql-ruby versions containing breaking changes.

  1. Change schemas option default

Before: Your application's GraphQL schemas have to be explicitly provided through schemas option to be instrumented.

Now: GraphQL schemas is no longer required to be explicitly provided to schemas option. By default, every schema is instrumented.


Note: 1.13.x is only partially support(only class-based schema)
Note: Integration supported versions will be updated once graphql-ruby releases our patch
Note: Doc will be updated once graphql-ruby releases our patch

@github-actions github-actions bot added integrations Involves tracing integrations tracing labels Jan 19, 2024
@TonyCTHsu TonyCTHsu changed the base branch from 2.0 to tonycthsu/enable-graphql-test January 19, 2024 21:15
@TonyCTHsu TonyCTHsu marked this pull request as ready for review January 19, 2024 21:18
@TonyCTHsu TonyCTHsu requested review from a team as code owners January 19, 2024 21:18
Rakefile Outdated
@@ -88,7 +88,7 @@ TEST_METADATA = {
'graphql-2.1' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ✅ 3.1 / ❌ 3.2 / ❌ 3.3 / ❌ jruby',
'graphql-2.0' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ✅ 3.1 / ❌ 3.2 / ❌ 3.3 / ❌ jruby',
'graphql-1.13' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ✅ 3.1 / ❌ 3.2 / ❌ 3.3 / ❌ jruby',
'graphql-1.12' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ✅ 3.1 / ❌ 3.2 / ❌ 3.3 / ❌ jruby',
# 'graphql-1.12' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ✅ 3.1 / ❌ 3.2 / ❌ 3.3 / ❌ jruby',
Copy link
Member

Choose a reason for hiding this comment

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

We can just remove the line if we are dropping support for graphql 1.12.

Rakefile Outdated
Comment on lines 88 to 91
'graphql-2.1' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ✅ 3.1 / ❌ 3.2 / ❌ 3.3 / ❌ jruby',
'graphql-2.0' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ✅ 3.1 / ❌ 3.2 / ❌ 3.3 / ❌ jruby',
'graphql-1.13' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ✅ 3.1 / ❌ 3.2 / ❌ 3.3 / ❌ jruby',
'graphql-1.12' => '❌ 2.1 / ❌ 2.2 / ❌ 2.3 / ❌ 2.4 / ❌ 2.5 / ❌ 2.6 / ❌ 2.7 / ❌ 3.0 / ✅ 3.1 / ❌ 3.2 / ❌ 3.3 / ❌ jruby',
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 surprising -- why can't we test graphql on other Ruby versions? 🤔 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our 2.0 release contains breaking changes that makes existing graphql-ruby code not functional

Copy link
Member

@ivoanjo ivoanjo Jan 22, 2024

Choose a reason for hiding this comment

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

This matrix is for Ruby versions. I don't quite understand the connection between our graphql changes and why is it ok to test with Ruby 3.1, but not 3.2 or 2.7?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was using 3.1 as an example and will update the matrix with other Ruby versions once the changes are confirmed.

Also, the dependency should not be installing from my fork but the official gem release. 😄

Copy link
Member

Choose a reason for hiding this comment

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

Got it -- thanks for clarifying 🙇

@TonyCTHsu TonyCTHsu self-assigned this Jan 24, 2024
@TonyCTHsu TonyCTHsu changed the base branch from tonycthsu/enable-graphql-test to 2.0 January 25, 2024 20:39
@TonyCTHsu TonyCTHsu force-pushed the tonycthsu/graphql-breaking-changes branch from 6d5ecf9 to ae1ec81 Compare January 25, 2024 21:29
@TonyCTHsu TonyCTHsu merged commit 72f7307 into 2.0 Jan 26, 2024
151 checks passed
@TonyCTHsu TonyCTHsu deleted the tonycthsu/graphql-breaking-changes branch January 26, 2024 15:12
@TonyCTHsu TonyCTHsu linked an issue Jan 30, 2024 that may be closed by this pull request
@TonyCTHsu TonyCTHsu added this to the 2.0 milestone Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0 integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GraphQL tracing with Rails 7.0
3 participants