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: download introspection schema when apiId is passed #684

Merged
merged 3 commits into from
Sep 13, 2023

Conversation

dpilch
Copy link
Member

@dpilch dpilch commented Sep 5, 2023

Description of changes

  • download introspection schema when apiId is passed
amplify codegen add --apiId <api-id> --region <region>

When --apiId is set, the CLI will attempt to download the introspection schema to ./schema.json. When no --region is supplied us-east-1 is used. If the api cannot be found the CLI will prompt the customer to try a different region.

✖ Getting API details
AppSync API was not found in region us-east-1
? Do you want to choose a different region Yes
? Choose AWS Region (Use arrow keys)
❯ US East (N. Virginia) 
  US East (Ohio) 
  US West (N. California) 
  US West (Oregon) 
  EU (Stockholm) 
  EU (Milan) 
  EU (Ireland) 
(Move up and down to reveal more choices)

An additional change (#689) is needed after this PR. The customer can end in a broken state if:

  1. amplify codegen add --apiId <api-id>
  2. rm -rf schema.json
  3. amplify codegen

There is no way the customer can use the codegen CLI to download the introspection schema.

$ amplify-dev codegen types
⠋ Generating🛑 ENOENT: no such file or directory, open '/Users/dppilche/amplify/apps/testnoamplify/schema.json'

Resolution: Please report this issue at https://github.com/aws-amplify/amplify-cli/issues and include the project identifier from: 'amplify diagnose --send-report'
Learn more at: https://docs.amplify.aws/cli/project/troubleshooting/
⠹ Generating^Aborted!
$ amplify-dev codegen statements 
Cannot find GraphQL schema file: /Users/dppilche/amplify/apps/testnoamplify/schema.json
$ amplify codegen 
Please download the schema.graphql or schema.json and place in /Users/dppilche/amplify/apps/testnoamplify before adding codegen when not in an amplify project

Each of these commands need to initialize a download when outside an amplify project if apiId is set in .graphqconfig.yml.

Issue #, if available

N/A

Description of how you validated changes

  • Unit testing
  • E2E tests to come in later PR

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)

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

@@ -38,9 +38,9 @@ async function add(context, apiId = null, region = 'us-east-1') {
}

const schemaPath = ['schema.graphql', 'schema.json'].map(p => path.join(process.cwd(), p)).find(p => fs.existsSync(p));
if (withoutInit && !schemaPath) {
if (withoutInit && !(apiId || schemaPath)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

In the case that schema.json is already present when running amplify codegen add, the schema.json will be overwritten with no warning.

@@ -123,6 +123,8 @@ async function add(context, apiId = null, region = 'us-east-1') {
} else {
schema = getSDLSchemaLocation(apiDetails.name);
}
} else if (apiDetails) {
schema = await downloadIntrospectionSchemaWithProgress(context, apiDetails.id, 'schema.json', region);
Copy link
Member Author

Choose a reason for hiding this comment

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

Using introspection schema because the utility function already exists, but we could change to sdl schema.

@dpilch dpilch force-pushed the codegen-add-region branch from 856d465 to aff6b45 Compare September 8, 2023 18:01
@dpilch dpilch marked this pull request as ready for review September 8, 2023 18:04
@dpilch dpilch requested a review from a team as a code owner September 8, 2023 18:04
@dpilch dpilch changed the base branch from codegen-add-region to main September 8, 2023 20:05
@dpilch dpilch changed the base branch from main to codegen-add-region September 8, 2023 20:05
Base automatically changed from codegen-add-region to noinit-codegen September 12, 2023 17:23
@dpilch dpilch merged commit cc8088f into noinit-codegen Sep 13, 2023
@dpilch dpilch deleted the codegen-apiid branch September 13, 2023 21:31
alharris-at added a commit that referenced this pull request Sep 19, 2023
…set up locally. (#702)

* feat: codegen add --region (#683)

* fix: download introspection schema when apiId is passed (#684)

* fix: codegen --apiId broken state (#689)

* chore: get existing e2e tests passing (#704)

* feat: add e2e tests for codegen add when in a non-amplify project (#705)

* feat: add e2e tests for codegen add when in a non-amplify project

* chore: add some additional test cases

* noinit codegen from graphqlconfig (#706)

* fix: add missing awaits

* chore: fail typegen if no queries are found

---------

Co-authored-by: Dane Pilcher <[email protected]>
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.

3 participants