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

Support serverless-appsync-plugin v2 #183

Merged
merged 18 commits into from
Jan 24, 2024

Conversation

horiuchi
Copy link
Collaborator

@horiuchi horiuchi commented Mar 22, 2023

This PR is a rewrite to work this plugin with serverless-appsync-plugin version 2.
This change is incompatible with previous version because the serverless-appsync-plugin configuration is not compatible with version 1.

And, I have rewritten it in TypeScript. The existing functionality is still there.

#178

@horiuchi horiuchi changed the title Support typescript and serverless-appsync-plugin v2 [WIP] Support typescript and serverless-appsync-plugin v2 Mar 23, 2023
@horiuchi horiuchi marked this pull request as draft March 23, 2023 09:42
Rewrite by TypeScript and update to support `serverless-appsync-plugin` for v2
@horiuchi horiuchi changed the title [WIP] Support typescript and serverless-appsync-plugin v2 Support typescript and serverless-appsync-plugin v2 Mar 24, 2023
@horiuchi horiuchi marked this pull request as ready for review March 24, 2023 02:47
@horiuchi
Copy link
Collaborator Author

I can run serverless-appsync-plugin v2 in my local environment with serverless-offline.

@horiuchi horiuchi changed the title Support typescript and serverless-appsync-plugin v2 Support serverless-appsync-plugin v2 Mar 24, 2023
@bboure
Copy link
Member

bboure commented Mar 24, 2023

Thank you @horiuchi I will have a look at this asap.

I was considering deprecating this package though and offer other solutions.
I'd be happy to discuss this with you and understand your use cases

@horiuchi
Copy link
Collaborator Author

Thank you @bboure for the reply.

I don't seem to have any problems with my usage, but what concerns do you have?

@bboure
Copy link
Member

bboure commented Mar 24, 2023

No concern at all.
Over the time, I moved toward a strategy where I test in the cloud vs a simulator.
Simulators can be unreliable.

My goal is to make the serverless-appsync-plugin itself evolve toward this strategy.
For example, by introducing a feature similar to sls deploy lambda or SAM accelerate for a faster deployments and feedback loop.

@horiuchi
Copy link
Collaborator Author

I see.
It would certainly be ideal if the `serverless-appsync-plugin' could be used to easily run and debug in a remote environment.
It is inevitable that there will be some differences in the behavior between the simulator and the actual environment.

But in practice, the only way to debug AppSync is to use serverless offline.
So I made this change because I need it and use it myself.

However, I think there is an option not to merge this change because it is incompatible with the previous version.
In that case, I would consider publishing it under a different name.

@bboure
Copy link
Member

bboure commented Mar 24, 2023

I think we can version bump this package to v2 with your changes (skipping v1, to align it with the version of the main plugin).

@horiuchi
Copy link
Collaborator Author

@bboure When will this PR be merged and published to npm?

@Hideman85
Copy link

We also depends on this here, do you think you could release this?

@Hideman85
Copy link

@horiuchi I tried the source branch on my local and I'm getting

AppSync Simulator: TypeError: Cannot read properties of undefined (reading 'name')

First how can I turn on verbose debug? SLS_DEBUG=* and DEBUG=* doesnt do anything for me 😕

@horiuchi
Copy link
Collaborator Author

@Hideman85 Thank you for trying this.

Setting SLS_DEBUG=* as an environment variable will increase the log output.

However, this error probably occurs when loading the config, so there is no additional information.
Could you try running serverless in a debugger to see where the exception occurs?
Or, if you can give us a serverless config file that reproduces the situation, we can find the cause.

@Hideman85
Copy link

Hey thanks for getting back to me. I checked and the logging is really poor even with SLS_DEBUG=*

First issue name of undefined was on the official one (try to read custom.appSync.name) then your fork and update solve the config point. I got similar issue map of undefined now the resolvers are pipeline by default and try to do functions.map.
So the logging is supper annoying for debugging and should be increased, at least would be great to show what is trying to process.

Last point, the update to support v2 is incomplete, v2 allow inline data source and here again, the config is fine it fails at starting up the appsync simulator with cannot find dataSource xxx.

In the end I ended up to refactor our serverless.yml to ts version with helper function to create the resources correctly, so then I handle validation myself at config before anything start up. It becomes a lot clearer than the errors thrown today. Wasnt really fun but now our team is safer and ready to go v2 with JsResolvers (good thing is we could support ourself bundling on the fly the js resolvers then).

@Hideman85
Copy link

@bboure Sorry to disturb you, have you considered merging this fix for now? Right now I'm using the fork in our flow and this add quite some overhead in our process. This would be awesome to have this released 🙏


exports[`resolveConfiguration should generate a valid config 2`] = `
{
"apiKey": "0123456789",

Choose a reason for hiding this comment

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

no multiple key ?

@mstoyanovv
Copy link

I am getting 'Duplicate "graphql" modules cannot be used at the same time' error when using the changes from this pr. Anyone else facing the same issue?

@dsiebes
Copy link

dsiebes commented Jul 12, 2023

@horiuchi Thank you for the generous work here.
@bboure Thank you for this package and all of the support and honest feedback on its future.

I have also tried this branch and ran in to an issue. I will try to come back here and provide a fix or commit but time is limited at this moment so I will just report a bug. I wonder if there is interest in having this version funded?

Although simulator is not perfect, it provides us with a way to debug resolvers locally which is much more efficient than deploying and logging. This is a very helpful package. Perhaps there are ways to do it in the cloud; it seems like a mystery to me.

output

SLS_DEBUG=* sls offline start
...
Error: Missing response mapping template Missing mapping template 
    at new AppSyncBaseResolver (/.../backend/graphql-api/node_modules/amplify-appsync-simulator/lib/resolvers/base-resolver.js:13:19)

serverless.yml (relevant example)

resolvers:
  Query.getThing:
    kind: PIPELINE
    functions:
      - firstTask
      - secondTask
      
 ...
 pipelineFunctions:
  firstTask:
    dataSource: DataSourceOne
    code: src/taskOne.ts

  secondTask:
    dataSource: DataSourceTwo
    code: src/taskTwo.ts

@Karjan1
Copy link

Karjan1 commented Sep 5, 2023

@horiuchi Thank you for the generous work here. @bboure Thank you for this package and all of the support and honest feedback on its future.

I have also tried this branch and ran in to an issue. I will try to come back here and provide a fix or commit but time is limited at this moment so I will just report a bug. I wonder if there is interest in having this version funded?

Although simulator is not perfect, it provides us with a way to debug resolvers locally which is much more efficient than deploying and logging. This is a very helpful package. Perhaps there are ways to do it in the cloud; it seems like a mystery to me.

output

SLS_DEBUG=* sls offline start
...
Error: Missing response mapping template Missing mapping template 
    at new AppSyncBaseResolver (/.../backend/graphql-api/node_modules/amplify-appsync-simulator/lib/resolvers/base-resolver.js:13:19)

serverless.yml (relevant example)

resolvers:
  Query.getThing:
    kind: PIPELINE
    functions:
      - firstTask
      - secondTask
      
 ...
 pipelineFunctions:
  firstTask:
    dataSource: DataSourceOne
    code: src/taskOne.ts

  secondTask:
    dataSource: DataSourceTwo
    code: src/taskTwo.ts

@dsiebes Did you figure out that issue?

@dsiebes
Copy link

dsiebes commented Sep 5, 2023

@Karjan1
We went a different path and wrote tests with Jest + Appsync SDK to evaluate our resolvers. It isn't ideal, but simulating locally with serverless framework was looking like an uphill battle overall. SAM might work for this, and there are other third party tools to do serverless local. But for now, we just have what are essentially integration tests that we can run locally and/or in our ci/cd pipelines.

@liam-ot
Copy link

liam-ot commented Sep 19, 2023

@bboure will this be merged and deployed? I understand you wish to deprecate but a lot of people (including the company i work for) rely very heavily on it

Thanks for all the work you and your contributors have put in! : )

@giraudvalentin
Copy link

is the "code" feature with js runtime working ?

@bboure
Copy link
Member

bboure commented Sep 26, 2023

Sorry all for not taking enough consideration on this topic.

The AppSync ecosystem evolves very fast and I'm having trouble keeping up with everything :-(

@horiuchi have you been maintaining this PR (or anything else) to keep it in sync with both the changes that happened in the v2 of the main plugin and the updates that amplify received?
I see that the latest commits are from April.

I'd be happy to help release a v2 of this plugin to unlock everyone who is blocked by this.

@horiuchi
Copy link
Collaborator Author

No updates have been made since this PR was created.
I have not followed AppSync updates either.

However, if there is any problem with merging this PR, I would like to support it.

@bboure
Copy link
Member

bboure commented Sep 26, 2023

Thank you for your update @horiuchi

I will have a look again at this, but I have not followed updates on the amplify simulator which is a big dependency of this package.

Have you been using this in your day-to-day work? Do. Do you have enough confidence that it works?

I'd suggest we open a v2 (or v1 since it is currently in v0) as a beta on the '@next` channel and start collecting feedback.

@horiuchi
Copy link
Collaborator Author

I don't use it myself at all these days.

I agree with the release as a beta.
It would be worth it, at least to solve the problem of serverless-appsync-plugin v2 not working at all.

@plezan
Copy link
Collaborator

plezan commented Sep 26, 2023

Thanks for your work ! I still use the v1 in my day to day work. We plan to switch to the v2 and I would be happy to get involved for the v2 of the simulator.

@plezan
Copy link
Collaborator

plezan commented Nov 13, 2023

Have you been using this in your day-to-day work? Do. Do you have enough confidence that it works?

I've been using it in my day-do-day work for about a month, it works properly except for pipeline functions.

I've opened a PR to @horiuchi 's repo to fix that issue
horiuchi#4

@plezan
Copy link
Collaborator

plezan commented Dec 21, 2023

I'd be happy to help release a v2 of this plugin to unlock everyone who is blocked by this.

@bboure If the v2 still planned, can we help one way or another ?

@bboure
Copy link
Member

bboure commented Dec 21, 2023

@plezan Unfortunately, I was not able to follow up on this.

If that helps, I can invite a few volunteers as maintainers to this repo to help keep the ball rolling.

@plezan
Copy link
Collaborator

plezan commented Dec 21, 2023

@bboure I would be happy to help keeping this repo moving forward.

@bboure
Copy link
Member

bboure commented Dec 29, 2023

@plezan I have added you and @horiuchi as maintainers.

This project uses semantic-release. (Also see here)

I'd recommend creating a pre-release (next) for this PR.

If you have any questions, let me know.

@horiuchi
Copy link
Collaborator Author

horiuchi commented Jan 4, 2024

Thank you for inviting me to be a maintainer.

But What should I do as a maintainer?
I don't use serverless or appsync at all these days, so I don't have an immediate environment to try it out.

@bboure
Copy link
Member

bboure commented Jan 4, 2024

@horiuchi Don't worry. There is no obligation of anything.
If and when you have a use case to pick this back up, feel free to complete it.
Otherwise, I'm sure others will

@plezan plezan changed the base branch from master to beta January 24, 2024 00:23
@plezan plezan merged commit d2a33a9 into serverless-appsync:beta Jan 24, 2024
1 check passed
Copy link

🎉 This PR is included in version 1.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@bboure
Copy link
Member

bboure commented Jan 24, 2024

Amazing! Good job @plezan ! 🙏

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.

9 participants