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

AppSyncApi: create resolver from the api not datasource #418

Merged
merged 1 commit into from Jun 10, 2021

Conversation

mmccall10
Copy link
Contributor

close #417

Creating a resolver from a data source results in the inability to update that resolver data source later.
aws/aws-cdk#12973 was merged to revert allowing resolvers to be created against the API.

This change creates the resolver from the AppSync graphqlApi API which allows you to update the resolver in place.

@fwang fwang added the bug Something isn't working label Jun 8, 2021
@fwang
Copy link
Contributor

fwang commented Jun 8, 2021

Hey @mmccall10 Is this a breaking change? Or will the resolvers get seamlessly re-created against the API?

@mmccall10
Copy link
Contributor Author

@fwang good point. Yes, this is breaking change. If the resolver has been created from the datasource (sst.Appsync construct) it would need to be removed then reattached. This is for the same reason you cannot update a datasource after the resolver has been created from a datasource.

If the resolver has been created from the api it's good.

Might be worth adding a flag/config on the construct? Maybe even separate methods ie addResolversFromDataSource or addResolversFromApi? sst can have it's opinion on what the default strategy is.

@fwang
Copy link
Contributor

fwang commented Jun 9, 2021

Yeah you are right. I was able to verify this PR fixes the "Only one resolver is allowed per field." issue.

Is there any reason for someone to attach resolvers to a data source?

@mmccall10
Copy link
Contributor Author

I feel there is no reason to attach it to the datasource by default at this time. The "only one resolver is allowed per field" has been a known CFN issue since 2018. Regardless the AppSync constructs in CDK are still considered experimental. And if sst is leveraging that, it's safe assume sst.Appsync construct is experimental too. Passing the warning on to sst users might be a good move, or leverage the Cfn constructs.

From AppSync cdk:
"The APIs of higher level constructs in this module are experimental and under active development. They are subject to non-backward compatible changes or removal in any future version. "

@fwang fwang changed the title Create AppSync resolver from the api not datasource AppSyncApi: create resolver from the api not datasource Jun 10, 2021
@fwang fwang merged commit 67a334a into sst:master Jun 10, 2021
@fwang fwang added the breaking label Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing an AppSync resolver results in Cloudformation error Only one resolver is allowed per field.
2 participants