feat: Support apiId(Multiple cloudformation stacks)

HumbleBeck commented 1 year ago

Closes #595

Hideman85 commented 1 year ago

@HumbleBeck I have the same need, first thank you for your work and this is already helping me. Now I have the following:

// serverless-compose.yml
services:
  service-root:
    path: serverless-definitions/service-root
  service-partA:
    path: serverless-definitions/service-partA
    dependsOn: service-root
    params:
      ParameterApiId: ${service-root.ParameterApiId}
  service-partB:
    path: serverless-definitions/service-partB
    dependsOn: service-root
    params:
      ParameterApiId: ${service-root.ParameterApiId}

And I'm getting this error in the other stacks:

Template format error: Unresolved resource dependencies [GraphQlSchema, ParameterApiId] in the Resources block of the template

Checkking the generated CF and getting on all the resolvers "DependsOn": ["GraphQlSchema"], but this resource is only on the root one

Do you think you can have a look at it? Or am I doing something wrong?

HumbleBeck commented 1 year ago

hey @Hideman85, indeed, I think I've missed this part thank you Just pushed a commit, it should fix it.

bboure commented 1 year ago

Thanks all for the great feedback and this PR

I will take everything into consideration and have a look at this as soon as I can

Hideman85 commented 1 year ago

@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 🙏

morficus commented 1 year ago

@bboure +1 for getting this merged in.

my use case is that I have a large AppSync API with v1 of this plugin. we are upgrading to v2 and, at the same time, splitting the API into "services" and using serverless-compose to manage them. being able to have an individual "service" update parts of the AppSync API would be phenomenal since it would make things more manageable and speed up deployments.

thank you @HumbleBeck for the awesome work 🙏

bboure commented 1 year ago

Sorry, I'll try to have a look at this over the weekend.

Meanwhile, AWS released support for merged APIs. I would like to have a look at it too

I think both are probably good to support but I'd like to understand how they fit together, etc.

Thanks for the work and your patience

morficus commented 1 year ago

@bboure OH WOW 😲 I did not hear about the new "merged API" functionality. AppSync finally has a solution for "schema federation" like Apollo. I'm going to have to dig into the blog post over the weekend.

Totally agree that it would be great to support both.

morficus commented 1 year ago

hey @bboure -- sorry to bug you again 😅 wondering if you were able to put some thought toward this PR or if you had a different approach in mind.

bboure commented 1 year ago

Thank you for your patience, I had a look at the PR. It looks good but I left some comments for improvements. Espscially on typing and validation.

A few more thoughts/questions:

should we allow exporting/importing data sources from other stacks? e.g. I create my data sources in a main stack (could be a single-table DynamoDB), and then I reference it from resolvers in other stacks. Right now, we use an intrinsic function to get the name, which will not work if it's in a different stack. I assume passing the value/name as-is should work.

The same goes for pipeline functions.

We also need to update the migration guide and remove the reference that using existing APIs is not supported since this is being re-introduced.
I'd really like to have a full guide on what this is for and when and how to use it.

Thank you all for the effort!

morficus commented 1 year ago

cc: @HumbleBeck ☝️ Check out the comments left on the PR.

morficus commented 1 year ago

I'd really like to have a full guide on what this is for and when and how to use it.

@bboure I can volunteer to help with this part.

should we allow exporting/importing data sources from other stacks?

my vote on this is no, because as you said, passing the name as-is works. also sharing the same data source between stacks smells like an anti-pattern (unless using single-table design with DynamoDB, which I have not seen people do when using AppSync)

alexandre-snr commented 1 year ago

should we allow exporting/importing data sources from other stacks?

I am interested in this feature, I'd vote yes for the DynamoDB single tables, I feel like this is a very important use case

HumbleBeck commented 1 year ago

Hey guys, Thanks for waiting. I was occupied with chores. I'll review the comments by EOW. Meanwhile, can I ask you, @morficus, to help me with documentation/guide? I'll drop this part from my PR.

DaRo0 commented 1 year ago

I'm really interested in this feature! we can't update our library until this is finished. Just out of curiosity, @HumbleBeck were you able to move forward with your PR? Thanks a lot everyone for the great effort!

BohdanShuliaka commented 1 year ago

@HumbleBeck +1 for getting this merged in.

bboure commented 1 year ago

What is the status of this @HumbleBeck ?

DaRo0 commented 8 months ago

@bboure @HumbleBeck did this fell under the table? I don't know if I can help somehow, besides having no knowledge at all

sid88in / serverless-appsync-plugin

feat: Support apiId(Multiple cloudformation stacks) #597