aws-samples / aws-serverless-ecommerce-platform

Serverless Ecommerce Platform is a sample implementation of a serverless backend for an e-commerce website. This sample is not meant to be used as an e-commerce platform as-is, but as an inspiration on how to build event-driven serverless microservices on AWS.
MIT No Attribution
1.11k stars 376 forks source link

TemplateURL to dlq.yaml wrong #139

Closed ljacobsson closed 4 years ago

ljacobsson commented 4 years ago

The TemplateURL to dlq.yaml steps up one folder too many. This is true for all templates pulling in dlq.yaml.

Changing to TemplateURL: ../shared/templates/dlq.yaml works

nmoutschen commented 4 years ago

Hey Lars!

Thanks a lot for opening this issue!

There's a reason why it goes one folder more than expected. After the build phase, all elements are copied into the {service}/build/ folder before the package phase. Thus the template will be in orders/build/ instead of orders/. Because of that, the TemplateURL needs to go back 2 folders (orders/build/../../shared/templates/dlq.yaml which will resolve to shared/templates/dlq.yaml).

ljacobsson commented 4 years ago

Thanks for clarifying :-)

I thought it was something like that going on, but couldn't find where it happened, but now I found it under tools/build.

I used this project when testing (I always do, it's great :-)) inclusion of nested stacks in cfn-diagram, but it kept breaking when it couldn't find the dlq.yaml

For developers contributing to the SAM template, but not familiar with the build procedure I think it gets a bit confusing to point to not-yet-existing paths.

nmoutschen commented 4 years ago

Fair point! I'll create a PR with some comments in the templates that use the DLQ explaining why the path is like that.

Also, happy to hear you're using this project for testing. 😄 I've used the diagram tool in evb-cli to demo EventBridge in this project by the way.

nmoutschen commented 4 years ago

I put some explanations around that in https://github.com/aws-samples/aws-serverless-ecommerce-platform/pull/144