serverless / compose

Orchestrate Serverless Framework in monorepos
https://serverless.com/framework/docs/guides/compose
MIT License
111 stars 15 forks source link

Add explicit error message when trying to deploy a single service using parameters #29

Closed adriencaccia closed 2 years ago

adriencaccia commented 2 years ago

Hey !

As said in the docs, using serverless deploy to deploy a service using parameters does not work:

However, if "service-a" uses ${param:xxx} to reference serverless-compose.yml parameters, then serverless deploy will not work. Indeed, ${param:xxx} cannot be resolved outside of Serverless Framework compose.

But the error message is not explicit, for example when running serverless deploy inside the consumer directory of the example:

yarn serverless deploy                  
Environment: linux, node 14.17.1, framework 3.7.4 (local), plugin 6.1.5, SDK 4.3.2
Docs:        docs.serverless.com
Support:     forum.serverless.com
Bugs:        github.com/serverless/serverless/issues

Error:
Cannot resolve serverless.yml: Variables resolution errored with:
  - Cannot resolve variable at "functions.consumer.events.0.sqs.arn": Value not found at "param" source

It would be great to have a message similar to the one in the docs !

mnapoli commented 2 years ago

Hey, thanks for opening this!

This could be tricky to improve the error message, because:

The only thing I think can be easy to improve would be to rephrase the message to:

- Value not found at "param" source
+ The parameter "xxx" cannot be found. Looked in CLI arguments, stage parameters [and Serverless Dashboard].

but this is only a marginal improvement 🤔

adriencaccia commented 2 years ago

Knowing for sure why the param can not be resolved is really tricky for sure :+1:

The rephrasing of the message you proposed is a good start though !

fargito commented 2 years ago

@mnapoli maybe the fact that params can come from different source can be a bit confusing for developers... Maybe we could use another keyword like importedParams ? In my opinion, this would have three benefits:

What do you think?

mnapoli commented 2 years ago

The goal with the "parameters" concept was to unify configuring a service. Instead of dealing with many different alternatives (env vars, .env, custom/self variables, ssm, CLI options…) we have 1 concept.

So if we start splitting again in different variable sources we move away from that goal.

Note that we also have issue #4 to discuss how we could resolve parameters from Serverless Compose (when running a serverless command).

@pgrzesik do you know if it's easy in Serverless Framework to change the error message to something like what's described in this comment?

pgrzesik commented 2 years ago

Hey @mnapoli - it would be possible but it's not necessarily "easy" as this error message is unified across all variable sources - it's not a custom one depending on the source.

mnapoli commented 2 years ago

Following internal discussions, let's try and aim for this error message in Serverless Framework:

The parameter "xxx" cannot be resolved.

Looked for a value in CLI arguments, stage parameters [and Serverless Dashboard].
If you are using Serverless Compose, make sure to run commands at the root of the project so that all parameters can be resolved.