iframe-proxy

braahyan · 2016-11-20T00:16:13Z

What did you implement:

Lambda environment variable support.

How did you implement it:

Added a new property to the function template object and then used Object.assign to merge the dictionaries from the provider object and the function object.

How can we verify it:

`serverless.yml`

service: environment-variables

provider:
  environment:
    provider_level: provider
  name: aws
  runtime: nodejs4.3
  cfLogs: true

functions:
  hello:
    environment:
      FUNCTION_LEVEL: function
    handler: handler.hello
    events:
      - http:
          integration: lambda
          path: hello
          method: get

`handler.js`

'use strict';

module.exports.hello = (event, context, callback) => {
  // Use this code if you don't use the http event with the LAMBDA-PROXY integration
  callback(null, {
    message: 'Go Serverless v1.0! Your function executed successfully!',
    event,
    context,
    env: process.env,
  });
};

Todos:

Write tests
Write documentation
Fix linting errors
Make sure code coverage hasn't dropped
Provide verification config/commands/resources
Enable "Allow edits from maintainers" for this PR
Change ready for review message below

Is this ready for review?: YES

austencollins · 2016-11-20T00:42:32Z

dougmoscrop · 2016-11-20T01:52:44Z

      newFunction.Properties.Description = functionObject.description;
    }

+    if (functionObject.environment) {


This entire if block can be represented as:

newFunction.Properties.Environment.Variables = Object.assign({}, this.serverless.service.provider.environment, functionObject.environment)

assign can take multiple arguments

assign will ignore undefined things
(also this would allow specifying env in the service that applies to all functions; as it stands this code would only assign service-level environment variables if the function also defines them)(

Thanks for the tip.

dougmoscrop · 2016-11-20T01:53:26Z

Might be worth validating keys of environment

pmuens · 2016-11-20T07:59:45Z

@braahyan Damn! That is so awesome! 👍 👏 🎉

Thank you very much for this! We'll review and test it thoroughly ASAP.

braahyan · 2016-11-20T18:51:15Z

This PR also gives plugins the ability to inject environment variables:

'use strict';

const BbPromise = require('bluebird');

class EnvironmentVariableInjector {
  constructor(serverless, options) {
    this.serverless = serverless;
    this.hooks = {
      'before:deploy:compileFunctions': () => BbPromise.bind(this)
        .then(this.beforeDeployResources)
    };
  }

  beforeDeployResources() {
    if(!this.serverless.service.provider.environment){
      this.serverless.service.provider.environment = {}
    }
    this.serverless.service.provider.environment["INJECTED_FROM_PLUGIN"] = "asdfdasdfasdfdf"
  }
}

module.exports = EnvironmentVariableInjector;

pmuens · 2016-11-20T19:25:58Z

Great @braahyan you've done an awesome job here! 👍 🎉
Thank you very much for this!

braahyan · 2016-11-20T19:33:28Z

I think an obvious enhancement would be to drop the Ref attributes from cloud watch into the environment a la https://github.com/rurri/serverless-resources-env

pmuens

Thank you very much for this PR! I really love the simplicity! Great job @braahyan 💯 🎉

I just added a comment that we might want to check if the environment property is given and then create the corresponding CloudFormation parts.

This will reduce the template size a little bit and removes the need to update all the function related template tests (as we need to add an empty property for the environment variables so that the tests succeed).

pmuens · 2016-11-21T07:30:27Z

      newFunction.Properties.Description = functionObject.description;
    }

+    newFunction.Properties.Environment.Variables = Object.assign(


Is there a way to create this only if an environment property is given on the function or provider level?

This way we won't have to add the empty property to every test (and reduce the template size a little bit).

pmuens · 2016-11-21T07:37:35Z

+
+    Object.keys(newFunction.Properties.Environment.Variables).forEach((key) => {
+      // I pulled this from the bash man pages
+      if (!key.match(/^[A-Za-z_][a-zA-Z0-9_]*$/)) {


Does this apply for the Lambda environment variables as well?

pmuens · 2016-11-21T08:10:58Z

@braahyan just a quick update.
We'll push a minor change atop of your PR and then merge it so that Serverless supports this ASAP!
Great job!

nikgraf · 2016-11-21T08:45:29Z

+```

-You can use [python-dotenv](https://github.com/theskumar/python-dotenv) to load files with environment variables. Those variables can be set during your CI process or locally and then packaged and deployed together with your function code.
+Or if you want to apply Environment Variable configuration to all functions in your service, you can add the configuration to the higher level `provider` object, and overwrite these service level config at the function level. For example:


I find the last part a bit confusing. It looks liked overwrite these service level config at the function level is part of the goal, but sometimes I want to have an environment variable available to all of my Lambdas

pmuens · 2016-11-21T08:46:53Z

Just pushed the minor updates and tested it again. Works fine!

GTM from my side!

nikgraf · 2016-11-21T08:47:50Z

Looks good to me 👍

nicka · 2016-11-21T08:49:15Z

Is it too early for 🍻 ?

pmuens · 2016-11-21T08:51:53Z

@nicka This will be in v1.2 thanks to @braahyan ! 🍻

nikgraf · 2016-11-21T09:10:05Z

@nicka don't think so, cheers dude

nicka · 2016-11-21T09:12:18Z

nikgraf · 2016-11-21T09:12:44Z

LOL

braahyan · 2016-11-21T14:24:31Z

Exciting!

braahyan · 2016-11-21T15:51:07Z

We should probably update the sample config that gets created with sls create!

pmuens · 2016-11-21T15:56:46Z

@braahyan Ah you mean the serverless.yml file from the template?

That's a good idea!

pmuens · 2016-11-21T16:27:01Z

@braahyan We'll add it to the templates in the next hrs. Thanks for the suggestion! 👍

stevecrozz · 2016-11-21T18:29:03Z

First of all, I appreciate serverless and all the contributors immensely and I don't want to minimize that.

But was there a place where discussion happened on this feature? This closes proposal #2673 (which had no substantive public discussion). I still don't even fully understand what's been implemented (still reading the commit). From the outside, it looks like there's a lot of time pressure to implement features as quickly as possible without taking the time to plan and discuss.

What I'm saying is, I'd like to participate in public design discussions or at least observe those discussions (even after the fact), but I'm not sure where/when that's happening. Is it happening?

pmuens · 2016-11-22T08:26:05Z

Hey @stevecrozz
thank you very much for your comment 👍
Environment variable support was a highly requested feature and we're just a few hrs. away of releasing v1.2 therefore we decided to merge this PR so that we get the first iteration of environment variable support out with v1.2.

We've added more docs for this feature in another PR and have also added an example in our Serverless examples repository (see: https://github.com/serverless/examples/tree/master/function-with-environment-variables).

This PR is just a starting point for environment variable support. We'd like to add enhancements soon so that it get's even more valuable. This means that we love to hear feedback and design proposal for future features / enhancements.

We've opened up a separate issue with the pending proposals so that we can continue the discussion there (#2764).

Thanks for bringing this up @stevecrozz 👍 . Feedback is one of the most important resources for us.

nikgraf · 2016-11-22T12:49:52Z

@stevecrozz I went through the whole thread and I also read your comment. We discussed the implementation. We also took into account your comment. I will share an example on how to implement the approach you mentioned here.

We believe the current implementation a very generic one we found to be applicable for various real-world use-cases. There are many how we can improve it.

So for once we would love to have your input there, but I'm also curious how you would like to see us dealing with new feature requests. How can we do better?

stevecrozz · 2016-11-22T17:14:40Z

If this gets released as part of v1.2, aren't we committed to maintaining support for it in its released form (backwards compatibility) for the long haul? If so, we're jumping into that commitment rather quickly.

This team is highly impressive in terms of how quickly these features can be implemented. But personally, I'd prefer to put more focus on stability of the framework and less on time-to-market. Where's the fire? Sure lots of users want environment variable support (I'm one of them), but would I want it one way in 1.2 and then a different backwards incompatible way in 1.3? Not at all. But maybe this is where I'm out of step with the rest of the community.

One last thought. This is maybe not the most appropriate place for me to have started a meta-discussion on process and project philosophy. Where can we have these discussions? I don't see a public mailing list and I've had these larger questions with no place to ask them. If there's nothing like this, I'd suggest setting something up.

nikgraf · 2016-11-22T17:36:31Z

Stability is definitely our highest priority. We follow semver so until a 2.0 is released nothing is going to go away only added or fixed. So yes, whatever we have now will be reported for quite some time as we don't plan 2.0 anytime soon. I'm not sure if we move to fast or too slow. Some people wanted to have the feature asap, some want to discuss for a while. It's a trade-off we needed to make …
The best we can learn for the future is very concrete what exactly should have happened from your perspective.

The whole team is hanging out in the #serverless_framework channel here: https://wt-serverless-seattle.run.webtask.io/serverless-forum-signup
Happy to chat there :)

braahyan · 2016-11-22T17:42:59Z

bryan pedlar added 5 commits November 19, 2016 18:35

added environment variable support

94d93e3

added documentation around environment variable configuration

07f8a5b

fixed linting errors

d4b16a1

fixed more linting errors

3fc632f

fixed yet more linting errors

e79bb27

dougmoscrop reviewed Nov 20, 2016

View reviewed changes

bryan pedlar added 3 commits November 19, 2016 21:38

global environment variables are added appropriately

1f1c63d

added validation of environment variable names

ca41f63

fixed linting issues

e381e9e

pmuens added the status/0-triage label Nov 20, 2016

pmuens modified the milestone: 1.2 Nov 20, 2016

pmuens added priority/P0 and removed status/0-triage labels Nov 20, 2016

pmuens suggested changes Nov 21, 2016

View reviewed changes

pmuens added status/2-code-review and removed status/1-design-review labels Nov 21, 2016

pmuens reviewed Nov 21, 2016

View reviewed changes

pmuens mentioned this pull request Nov 21, 2016

Add integration test for environment variable support #2751

Closed

pmuens added 2 commits November 21, 2016 09:22

Only create environment variables if config is given

2d390ed

Add additional tests

26ff0fa

nikgraf reviewed Nov 21, 2016

View reviewed changes

pmuens approved these changes Nov 21, 2016

View reviewed changes

pmuens added status/4-merge and removed status/2-code-review labels Nov 21, 2016

nikgraf merged commit 71eb5a1 into serverless:master Nov 21, 2016

nikgraf mentioned this pull request Nov 21, 2016

Design Environment Variable Support #2673

Closed

braahyan deleted the lambda-environment-variables branch November 21, 2016 14:53

braahyan restored the lambda-environment-variables branch November 21, 2016 14:53

braahyan deleted the lambda-environment-variables branch November 21, 2016 15:02

pmuens mentioned this pull request Nov 21, 2016

Add environment variable config to templates #2761

Merged

5 tasks

ghost mentioned this pull request Jan 5, 2017

Add Serverless Framework standard-library/wire-o#3

Closed

dncrews mentioned this pull request Feb 15, 2017

Please use proper semantic versioning #3252

Closed

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

braahyan commented Nov 20, 2016 • edited by pmuens Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What did you implement:

How did you implement it:

How can we verify it:

serverless.yml

handler.js

Todos:

Uh oh!

austencollins commented Nov 20, 2016

Uh oh!

dougmoscrop Nov 20, 2016

Choose a reason for hiding this comment

Uh oh!

braahyan Nov 20, 2016

Choose a reason for hiding this comment

Uh oh!

dougmoscrop commented Nov 20, 2016

Uh oh!

pmuens commented Nov 20, 2016

Uh oh!

braahyan commented Nov 20, 2016

Uh oh!

pmuens commented Nov 20, 2016

Uh oh!

braahyan commented Nov 20, 2016

Uh oh!

pmuens left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmuens Nov 21, 2016

Choose a reason for hiding this comment

Uh oh!

pmuens Nov 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmuens commented Nov 21, 2016

Uh oh!

nikgraf Nov 21, 2016

Choose a reason for hiding this comment

Uh oh!

pmuens commented Nov 21, 2016

Uh oh!

nikgraf commented Nov 21, 2016

Uh oh!

nicka commented Nov 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pmuens commented Nov 21, 2016

Uh oh!

nikgraf commented Nov 21, 2016

Uh oh!

nicka commented Nov 21, 2016

Uh oh!

nikgraf commented Nov 21, 2016

Uh oh!

braahyan commented Nov 21, 2016

Uh oh!

braahyan commented Nov 21, 2016

Uh oh!

pmuens commented Nov 21, 2016

Uh oh!

pmuens commented Nov 21, 2016

Uh oh!

stevecrozz commented Nov 21, 2016

Uh oh!

pmuens commented Nov 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikgraf commented Nov 22, 2016

Uh oh!

stevecrozz commented Nov 22, 2016

Uh oh!

nikgraf commented Nov 22, 2016

braahyan commented Nov 20, 2016 •

edited by pmuens

Loading

`serverless.yml`

`handler.js`

pmuens left a comment •

edited

Loading

pmuens Nov 21, 2016 •

edited

Loading

nicka commented Nov 21, 2016 •

edited

Loading

pmuens commented Nov 22, 2016 •

edited

Loading