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

fix: disallow Gen 1 Patterns in GraphQL SDL schema. #1719

Closed
wants to merge 4 commits into from
Closed

Conversation

dpilch
Copy link
Member

@dpilch dpilch commented Jul 5, 2024

Problem

Customers can use Gen 1 patterns in GraphQL SDL string schema. This is not intentional.

Issue number, if available: N/A

Changes

Disallows usage of specific gen 1 patterns. See aws-amplify/amplify-category-api#2670 for list.

Corresponding docs PR, if applicable: aws-amplify/docs#7793

Validation

  • Unit testing

Checklist

  • If this PR includes a functional change to the runtime behavior of the code, I have added or updated automated test coverage for this change.
  • If this PR requires a change to the Project Architecture README, I have included that update in this PR.
  • If this PR requires a docs update, I have linked to that docs PR above.
  • If this PR modifies E2E tests, makes changes to resource provisioning, or makes SDK calls, I have run the PR checks with the run-e2e label set.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

changeset-bot bot commented Jul 5, 2024

🦋 Changeset detected

Latest commit: 2d115d9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@aws-amplify/backend-data Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dpilch dpilch marked this pull request as ready for review July 15, 2024 21:15
@dpilch dpilch requested review from a team as code owners July 15, 2024 21:15
Copy link
Contributor

@edwardfoyle edwardfoyle left a comment

Choose a reason for hiding this comment

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

Couple small comments. My main concern is that this is a breaking change even if the product team doesn't want to release it as such. Do we have data around how many customers have a gen 1 schema in gen 2? Should we print a warning saying that this behavior will be removed in a future major version instead?

schema,
});
assert.throws(() => dataFactory.getInstance(getInstanceProps), {
message: 'Failed to instantiate data construct',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we throw an error message that explains what is not supported and how to fix it?

@@ -0,0 +1,5 @@
---
'@aws-amplify/backend-data': patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'@aws-amplify/backend-data': patch
'@aws-amplify/backend-data': minor

@dpilch dpilch marked this pull request as draft August 7, 2024 14:32
@dpilch dpilch closed this Aug 9, 2024
@rtpascual rtpascual deleted the allow-gen1 branch November 25, 2024 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants