fix(lambda): replace node 8 usages, allow node 10 inline assets #4655

nmussy · 2019-10-23T23:22:46Z

set supportsInlineCode to true for Runtime.NODEJS_10_X
add Runtime.NODE_LATEST, currently aliasing Runtime.NODEJS_10_X
replace all usages of Node 8 and 10 with Runtime.NODE_LATEST

Before merging, aws-cloudformation-coverage-roadmap#80 is still "Coming soon", we need to wait for it to be implemented

Alternative: It might be more maintainable to add "latest/stable" runtimes instead, aliasing the preferred version (e.g. Runtime.NODE_STABLE pointing to NODE_10_X)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

mergify · 2019-10-23T23:23:23Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

aws-cdk-automation · 2019-10-23T23:59:46Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-10-24T00:20:27Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-10-24T00:47:55Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb

I like the NODE_LATEST idea. @nija-at @rix0rrr @RomainMuller what do you guys think?

packages/@aws-cdk/aws-lambda/lib/runtime.ts

This reverts commit 0a7bd07.

This reverts commit 00d7ebf.

aws-cdk-automation · 2019-10-26T10:47:11Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-10-26T11:15:51Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-10-26T11:28:23Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-10-26T12:24:04Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-10-26T12:38:54Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-10-26T13:33:04Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

nmussy · 2019-10-26T13:42:55Z

Could a team member add a "do-not-merge" tag to this PR, waiting for aws-cloudformation-coverage-roadmap#80 to ship?

Thanks!

aws-cdk-automation · 2019-10-26T13:53:33Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-10-26T14:41:12Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

nija-at · 2019-11-13T19:14:04Z

It looks like CloudFormation now supports inline code for Nodejs10 Lambda functions (tried these out myself)

nmussy · 2019-11-13T19:38:27Z

@nija-at Great, thanks for letting me know! I'll merge master and see if there are new usages, but this should be good to go

nmussy · 2019-11-13T19:44:44Z

It should be good to go 👍

aws-cdk-automation · 2019-11-13T19:52:22Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-11-13T20:19:56Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

nija-at · 2019-11-14T10:07:13Z

I like the NODE_LATEST idea. @nija-at @rix0rrr @RomainMuller what do you guys think?

I have a couple of concerns around NODE_LATEST -

Firstly, does node guarantee that future proofing, i.e., all node8 scripts are valid node10, and so on? If not, we'll be breaking customers (at runtime which is worse) whenever we change NODE_LATEST.

Secondly, it is entirely within Lambda's contract and CloudFormation's contract to modify or stop supporting a feature that existed until one major version language to the next.

A good example is where, for a significant period of time, CloudFormation supported inline code for Node8 but not for Node10. If we had swiched NODE_LATEST during this time, we would have broken customers.

While this was a matter of time, I can other potential situations where such compatibility features and not available for a while; or upgrade is simply not possible without customers additional or modified attributes.

How do we handle such scenarios?

nija-at

Code changes themselves look good to me.

See my previous comment regarding NODE_LATEST.
For all places in the CDK, where the CDK is shipping its own Lambda function as part of its construct and impacted by this change, could we run at least one integ test to deploy the stack to a real account and execute the function at least once for correctness of functionality?

nija-at · 2019-11-14T10:11:28Z

design/code-asset-metadata.md

@@ -75,7 +75,7 @@ const resource = new serverless.CfnFunction(this, 'Func', {
    bucket: asset.s3BucketName,
    key: asset.s3ObjectKey
  },
-  runtime: 'nodejs8.10',
+  runtime: Runtime.NODEJS_LATEST,


Avoid changes to the design/ folder unless you're changing the design.

nija-at · 2019-11-14T11:05:00Z

Thinking a bit more on the NODE_LATEST feature - this assumes latest === recommended which I'm not sure will stay true always.

Given these questions around this and the fact that Node8 is EOL in December (coming around the corner), I suggest we pull out the NODE_LATEST change from here and have that as a separate discussion. (the former would be a chore and the latter would be a feat so it's announced in our change log)

nija-at · 2019-11-18T11:55:23Z

@nmussy - Given that Node8 deprecation is around the corner, I had to separate out the upgrade to Node10 from the NODE_LATEST changes.

I didn't have permissions to push on your branch (to update this PR) so I've pulled it out as a separate change #5075.
Apologies for the rush on this.

@nmussy

Upgrade all internal uses of node8.10 lambda runtime to node10.x. This includes all exported constructs that contain a lambda function within them. Update NODE_10_X runtime to allow inline code in CloudFormation, per aws-cloudformation/cloudformation-coverage-roadmap#80. Shout out to @nmussy who got this most of the way (#4655) close #4653, close #4642

nija-at · 2019-11-21T11:23:34Z

I got the upgrade to Node10 in through another PR - #5075

I'll be closing out on this one. Feel free to open another one if you want to discuss the NODE_LATEST feature but before that, do look at the questions raised in this PR.

feat(lambda): node 10 inline support

3af61c7

nmussy mentioned this pull request Oct 24, 2019

[lambda] Support inline lambda code for Node 10.x #4642

Closed

fix: replace all usages of node 8 with 10

00d7ebf

nmussy changed the title ~~feat(lambda): node 10 inline support~~ fix(lambda): replace node 8 usages, allow node 10 inline assets Oct 24, 2019

fix: whoops, didn't stage everything

0a7bd07

eladb reviewed Oct 24, 2019

View reviewed changes

packages/@aws-cdk/aws-lambda/lib/runtime.ts Show resolved Hide resolved

SomayaB assigned nija-at Oct 24, 2019

nmussy added 4 commits October 26, 2019 12:32

Revert "fix: whoops, didn't stage everything"

99295c9

This reverts commit 0a7bd07.

Revert "fix: replace all usages of node 8 with 10"

ea5302a

This reverts commit 00d7ebf.

Merge remote-tracking branch 'upstream/master' into node-10-inline

cdfe980

feat: NODEJS_LATEST, replace all uses of Node 8

33c4a54

fix: readonly

50dab7a

chore: update asset hashes

98b4e3d

chore: more asset updates

ced8f51

chore: more asset updates

8bfd28c

chore: more asset updates

08542ce

nmussy marked this pull request as ready for review October 26, 2019 13:41

nmussy requested review from garnaat, nija-at and rix0rrr as code owners October 26, 2019 13:41

nmussy requested review from RomainMuller, shivlaks, skinny85 and SoManyHs as code owners October 26, 2019 13:41

chore: also use NODE_LATEST for Node 10

cfb6706

fix: revert runtime replace

9f9871c

skinny85 added the pr/do-not-merge This PR should not be merged at this time. label Oct 28, 2019

nmussy added 2 commits November 13, 2019 20:42

Merge remote-tracking branch 'upstream/master' into node-10-inline

999a7b8

chore: replace new NODEJS_10 usages

a933d58

chore: add NODEJS_LATEST doc

9820e59

nija-at suggested changes Nov 14, 2019

View reviewed changes

nija-at mentioned this pull request Nov 18, 2019

feat: upgrade all uses of node8.10 lambda runtime to node10.x #5075

Merged

trivikr mentioned this pull request Nov 19, 2019

feat(lambda): node12.x, python3.8 and java11 runtimes #5107

Merged

nija-at closed this Nov 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(lambda): replace node 8 usages, allow node 10 inline assets #4655

fix(lambda): replace node 8 usages, allow node 10 inline assets #4655

nmussy commented Oct 23, 2019 •

edited

Loading

mergify bot commented Oct 23, 2019

aws-cdk-automation commented Oct 23, 2019

aws-cdk-automation commented Oct 24, 2019

aws-cdk-automation commented Oct 24, 2019

eladb left a comment

aws-cdk-automation commented Oct 26, 2019

aws-cdk-automation commented Oct 26, 2019

aws-cdk-automation commented Oct 26, 2019

aws-cdk-automation commented Oct 26, 2019

aws-cdk-automation commented Oct 26, 2019

aws-cdk-automation commented Oct 26, 2019

nmussy commented Oct 26, 2019

aws-cdk-automation commented Oct 26, 2019

aws-cdk-automation commented Oct 26, 2019

nija-at commented Nov 13, 2019 •

edited

Loading

nmussy commented Nov 13, 2019

nmussy commented Nov 13, 2019

aws-cdk-automation commented Nov 13, 2019

aws-cdk-automation commented Nov 13, 2019

nija-at commented Nov 14, 2019

nija-at left a comment

nija-at Nov 14, 2019

nija-at commented Nov 14, 2019 •

edited

Loading

nija-at commented Nov 18, 2019

nija-at commented Nov 21, 2019

fix(lambda): replace node 8 usages, allow node 10 inline assets #4655

fix(lambda): replace node 8 usages, allow node 10 inline assets #4655

Conversation

nmussy commented Oct 23, 2019 • edited Loading

mergify bot commented Oct 23, 2019

aws-cdk-automation commented Oct 23, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Oct 24, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Oct 24, 2019

AWS CodeBuild CI Report

eladb left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Oct 26, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Oct 26, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Oct 26, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Oct 26, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Oct 26, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Oct 26, 2019

AWS CodeBuild CI Report

nmussy commented Oct 26, 2019

aws-cdk-automation commented Oct 26, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Oct 26, 2019

AWS CodeBuild CI Report

nija-at commented Nov 13, 2019 • edited Loading

nmussy commented Nov 13, 2019

nmussy commented Nov 13, 2019

aws-cdk-automation commented Nov 13, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Nov 13, 2019

AWS CodeBuild CI Report

nija-at commented Nov 14, 2019

nija-at left a comment

Choose a reason for hiding this comment

nija-at Nov 14, 2019

Choose a reason for hiding this comment

nija-at commented Nov 14, 2019 • edited Loading

nija-at commented Nov 18, 2019

nija-at commented Nov 21, 2019

nmussy commented Oct 23, 2019 •

edited

Loading

nija-at commented Nov 13, 2019 •

edited

Loading

nija-at commented Nov 14, 2019 •

edited

Loading