#6031 Add the `sls:stage` variable #9296

mnapoli · 2021-04-13T11:50:24Z

${sls:stage} is a shortcut to:

${opt:stage, self:provider.stage, 'dev'}

This is a new attempt at solving #6031, replacing older PR #6049.

As a start, I'm only introducing ${sls:stage} for now, and opening this PR as WIP. Once we're happy with the implementation, I'll add region in this PR.

Closes: #6031

Note: npm run lint shows errors all over the codebase, so I'm ignoring it for now.

codecov · 2021-04-13T11:54:39Z

Codecov Report

Merging #9296 (cd66817) into master (8aa2757) will increase coverage by 0.06%.
The diff coverage is 90.87%.

❗ Current head cd66817 differs from pull request most recent head 3e64167. Consider uploading reports for the commit 3e64167 to get more accurate results

@@            Coverage Diff             @@
##           master    #9296      +/-   ##
==========================================
+ Coverage   86.86%   86.93%   +0.06%     
==========================================
  Files         316      316              
  Lines       11765    11827      +62     
==========================================
+ Hits        10220    10282      +62     
  Misses       1545     1545

Impacted Files	Coverage Δ
commitlint.config.js	`0.00% <ø> (ø)`
lib/classes/Variables.js	`99.73% <ø> (ø)`
.../cli/commands-schema/common-options/aws-service.js	`100.00% <ø> (ø)`
lib/cli/resolve-configuration-path.js	`96.42% <ø> (ø)`
lib/plugins/aws/customResources/index.js	`98.57% <ø> (ø)`
lib/plugins/aws/deploy/lib/extendedValidate.js	`100.00% <ø> (ø)`
lib/plugins/aws/deploy/lib/uploadArtifacts.js	`90.00% <ø> (ø)`
...ib/plugins/aws/package/lib/generateCoreTemplate.js	`95.34% <ø> (ø)`
...ib/plugins/aws/package/lib/saveCompiledTemplate.js	`100.00% <ø> (ø)`
lib/plugins/aws/package/lib/saveServiceState.js	`100.00% <ø> (ø)`
... and 47 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8aa2757...3e64167. Read the comment docs.

medikoo

@mnapoli great thanks for working on that!

What's exactly the status of it? Why exactly it's submitted as draft PR ?

mnapoli · 2021-04-14T11:06:01Z

Just wanted to make sure this is what you expected ^^

Since you seem to be good with this I'll add the region.

medikoo · 2021-04-14T11:07:58Z

Since you seem to be good with this I'll add the region.

@mnapoli region feels more AWS specific, so we decided to handle it with: #9005 as aws:region (for which PR is also very welcome!)

mnapoli · 2021-04-14T11:15:50Z

Oh 😄 This is why it was in draft. I'll remove the commit I just pushed then.

mnapoli · 2021-04-14T11:17:49Z

OK all good, ready for review, there's just ${sls:stage} now.

medikoo

Thank you @mnapoli looks very good! I've just suggested few improvements to tests.

medikoo · 2021-04-14T11:27:38Z

docs/providers/aws/guide/variables.md

@@ -134,6 +134,10 @@ functions:
      APIG_DEPLOYMENT_ID: ApiGatewayDeployment${sls:instanceId}
 ```

+**stage**
+
+The stage used by the Serverless CLI. The `${sls:stage}` variable is a shortcut for `${opt:stage, self:provider.stage}`.


It's technically a shortcut for ${opt:stage, self:provider.stage, "dev"}

medikoo · 2021-04-14T11:30:01Z

test/unit/lib/configuration/variables/sources/instance-dependent/get-sls.test.js

@@ -13,18 +13,24 @@ describe('test/unit/lib/configuration/variables/sources/instance-dependent/get-s
  let variablesMeta;
  let serverlessInstance;

-  before(async () => {
+  const initializeServerless = async (providerStage, cliOptionStage) => {


Let's update the signature to (configExt, options). It'll be more up to convention in other test files, and more portable for eventual future addition.

configExt should be a simply configuration extension to be applied via _.merge(config, configExt), and options, should be CLI options.

medikoo · 2021-04-14T11:31:19Z

test/unit/lib/configuration/variables/sources/instance-dependent/get-sls.test.js

-  });
+  };
+
+  beforeEach(async () => await initializeServerless());


This is invoked prior every test, even one which invokes initializeServerless() on it's own. So that's not clean.

Let's remove this, and simply invoke initializeServerless in context of it's

medikoo · 2021-04-14T11:31:58Z

test/unit/lib/configuration/variables/sources/instance-dependent/get-sls.test.js

-  });
+  };
+
+  beforeEach(async () => await initializeServerless());

  it('should resolve instanceId', () => {


We can reword to should resolve ${sls.instanceId}

medikoo · 2021-04-14T11:33:03Z

test/unit/lib/configuration/variables/sources/instance-dependent/get-sls.test.js


  it('should resolve instanceId', () => {
    if (variablesMeta.get('custom\0sls')) throw variablesMeta.get('custom\0sls').error;
    expect(typeof serverlessInstance.instanceId).to.equal('string');
    expect(configuration.custom.sls).to.equal(serverlessInstance.instanceId);
  });

+  it('should resolve ${sls:stage} to "dev" by default', () => {


I would do one it("should resolve ${sls:stage}") and in it's context simply test one by one, all three variants

mnapoli · 2021-04-19T16:39:44Z

@medikoo I have pushed changes that should address all your comments.

medikoo

Thank @mnapoli ! That looks great. I've just proposed few style simplifications, so code is simpler

medikoo · 2021-04-20T09:41:41Z

test/unit/lib/configuration/variables/sources/instance-dependent/get-sls.test.js

        missingAddress: '${sls:}',
        unsupportedAddress: '${sls:foo}',
        nonStringAddress: '${sls:${self:custom.someObject}}',
        someObject: {},
      },
    };
+    if (configExt !== undefined) {


Let's simplify this to if (configExt) {

medikoo · 2021-04-20T09:42:01Z

test/unit/lib/configuration/variables/sources/instance-dependent/get-sls.test.js

@@ -40,29 +47,61 @@ describe('test/unit/lib/configuration/variables/sources/instance-dependent/get-s
      configuration,
      variablesMeta,
      sources: { self: selfSource, sls: getSlsSource(serverlessInstance) },
-      options: {},
+      options: options !== undefined ? options : {},


Let's simplify to options || {}

medikoo · 2021-04-20T11:29:57Z

@mnapoli one more thing. I think it'll be nice to also update examples in our documentation, which use stage resolution (as e.g. ${opt:stage, self:provider.stage, 'dev'}, so those are replaced with simply ${sls:stage}.

I think those can easily be found by searching for opt:stage or self:provider:stage

This variable is a shortcut to: ``` ${opt:stage, self:provider.stage, "dev"} ```

mnapoli · 2021-04-20T12:11:18Z

@medikoo good idea! I just pushed again.

medikoo

Thank you @mnapoli that's great!

razor-x · 2021-04-21T01:18:31Z

Does opt:stage also cover the env var SERVERLESS_STAGE as well? E.g., I've been using ${opt:stage, env:SERVERLESS_STAGE, self:provider.stage}.

mnapoli · 2021-04-21T08:05:08Z

@razor-x a quick search in the codebase shows no result for SERVERLESS_STAGE, I don't think this is an official feature?

I also read here https://forum.serverless.com/t/process-env-serverless-stage-in-1-0/144:

In v0.5 I would be able to get the current stage during runtime with process.env.SERVERLESS_STAGE. This no longer appears to work in v1.0. Is there another way to do this?

It seems like it's no longer a feature indeed. I would say that no, this variable is not taken into account. I might be wrong though.

medikoo · 2021-04-21T12:32:01Z

@razor-x as @mnapoli pointed SERVERLESS_STAGE is not supported by Framework since v1.0.0.

So you can safely replace your notation with sls:stage

A shortcut to ``` ${opt:stage, self:provider.stage, "dev"} ```

nikhilpr23 · 2022-07-04T08:33:16Z

I understand that ${sls:stage} is a shortcut to ${opt:stage, self:provider.stage, 'dev'}. But if we want the default to be a stage other than dev, can we define that, and if so, how?

mnapoli · 2022-07-04T08:35:30Z

@nikhilpr23 please open a different discussion.

FYI you can set the default stage via provider.stage (see https://www.serverless.com/framework/docs/providers/aws/guide/services#serverlessyml). Let's continue the discussion elsewhere.

mnapoli marked this pull request as draft April 13, 2021 11:50

This was referenced Apr 13, 2021

Variables: Introduce access to resolved stage via sls:stage #6031

Closed

Add sls:stage and sls:region variables #6049

Closed

medikoo added the cat/variable label Apr 14, 2021

medikoo assigned mnapoli Apr 14, 2021

medikoo added the enhancement label Apr 14, 2021

medikoo reviewed Apr 14, 2021

View reviewed changes

mnapoli force-pushed the stage-alias branch from 04b3e5b to 1790003 Compare April 14, 2021 11:16

mnapoli marked this pull request as ready for review April 14, 2021 11:17

medikoo suggested changes Apr 14, 2021

View reviewed changes

mnapoli force-pushed the stage-alias branch from 1790003 to 6bcf00a Compare April 19, 2021 16:39

medikoo reviewed Apr 20, 2021

View reviewed changes

serverless#6031 Add the sls:stage variable

3e64167

This variable is a shortcut to: ``` ${opt:stage, self:provider.stage, "dev"} ```

mnapoli force-pushed the stage-alias branch from 6bcf00a to 3e64167 Compare April 20, 2021 12:11

medikoo approved these changes Apr 20, 2021

View reviewed changes

medikoo merged commit ef91ae1 into serverless:master Apr 20, 2021

mnapoli deleted the stage-alias branch April 20, 2021 14:50

medikoo mentioned this pull request Apr 23, 2021

File variable resolution happens before properties it depends on are resolved #7895

Closed

wwwehr pushed a commit to wwwehr/serverless that referenced this pull request Apr 27, 2021

feat(Variables): sls:stage variable (serverless#9296)

9df47ae

A shortcut to ``` ${opt:stage, self:provider.stage, "dev"} ```

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#6031 Add the `sls:stage` variable #9296

#6031 Add the `sls:stage` variable #9296

mnapoli commented Apr 13, 2021 •

edited by medikoo

codecov bot commented Apr 13, 2021 •

edited

medikoo left a comment

mnapoli commented Apr 14, 2021

medikoo commented Apr 14, 2021 •

edited

mnapoli commented Apr 14, 2021

mnapoli commented Apr 14, 2021

medikoo left a comment

medikoo Apr 14, 2021

medikoo Apr 14, 2021

medikoo Apr 14, 2021

medikoo Apr 14, 2021

medikoo Apr 14, 2021

mnapoli commented Apr 19, 2021

medikoo left a comment

medikoo Apr 20, 2021

medikoo Apr 20, 2021

medikoo commented Apr 20, 2021

mnapoli commented Apr 20, 2021

medikoo left a comment

razor-x commented Apr 21, 2021

mnapoli commented Apr 21, 2021

medikoo commented Apr 21, 2021

nikhilpr23 commented Jul 4, 2022

mnapoli commented Jul 4, 2022

#6031 Add the sls:stage variable #9296

#6031 Add the sls:stage variable #9296

Conversation

mnapoli commented Apr 13, 2021 • edited by medikoo

codecov bot commented Apr 13, 2021 • edited

Codecov Report

medikoo left a comment

Choose a reason for hiding this comment

mnapoli commented Apr 14, 2021

medikoo commented Apr 14, 2021 • edited

mnapoli commented Apr 14, 2021

mnapoli commented Apr 14, 2021

medikoo left a comment

Choose a reason for hiding this comment

medikoo Apr 14, 2021

Choose a reason for hiding this comment

medikoo Apr 14, 2021

Choose a reason for hiding this comment

medikoo Apr 14, 2021

Choose a reason for hiding this comment

medikoo Apr 14, 2021

Choose a reason for hiding this comment

medikoo Apr 14, 2021

Choose a reason for hiding this comment

mnapoli commented Apr 19, 2021

medikoo left a comment

Choose a reason for hiding this comment

medikoo Apr 20, 2021

Choose a reason for hiding this comment

medikoo Apr 20, 2021

Choose a reason for hiding this comment

medikoo commented Apr 20, 2021

mnapoli commented Apr 20, 2021

medikoo left a comment

Choose a reason for hiding this comment

razor-x commented Apr 21, 2021

mnapoli commented Apr 21, 2021

medikoo commented Apr 21, 2021

nikhilpr23 commented Jul 4, 2022

mnapoli commented Jul 4, 2022

#6031 Add the `sls:stage` variable #9296

#6031 Add the `sls:stage` variable #9296

mnapoli commented Apr 13, 2021 •

edited by medikoo

codecov bot commented Apr 13, 2021 •

edited

medikoo commented Apr 14, 2021 •

edited