Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(lambda-nodejs): non-deterministic runtime version #13893

Closed
zxkane opened this issue Mar 31, 2021 · 4 comments · Fixed by #14538
Closed

(lambda-nodejs): non-deterministic runtime version #13893

zxkane opened this issue Mar 31, 2021 · 4 comments · Fixed by #14538
Labels
@aws-cdk/aws-lambda Related to AWS Lambda @aws-cdk/aws-lambda-nodejs bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p1

Comments

@zxkane
Copy link
Contributor

zxkane commented Mar 31, 2021

❓ General Issue

The mutation default runtime of nodejs lambda looks like not a good practice.

aws-cdk/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts

Lines 33 to 40 in df5c133

/**
* The runtime environment. Only runtimes of the Node.js family are
* supported.
*
* @default - `NODEJS_14_X` if `process.versions.node` >= '14.0.0',
* `NODEJS_12_X` otherwise.
*/
readonly runtime?: lambda.Runtime;

Assuming a NodejsFunction lambda with default runtime, the unit test will fail if the CI uses matrix nodejs versions, such as 12.x, 14.x.

The Question

Environment

  • CDK CLI Version: 1.95.1
  • Module Version:
  • Node.js Version: 12.x / 14.x
  • OS:
  • Language (Version):

Other information
@zxkane zxkane added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Mar 31, 2021
@eladb
Copy link
Contributor

eladb commented May 2, 2021

I totally agree. Defaults should be completely deterministic. I think we should use nodejs 12.x as the default. @jogold can you please pick this up?

@eladb eladb added bug This issue is a bug. p1 and removed guidance Question that needs advice or information. labels May 2, 2021
@eladb eladb changed the title (lambda-nodejs): should not use mutable default runtime of nodejs lambda? (lambda-nodejs): non-deterministic runtime version May 2, 2021
@eladb eladb removed their assignment May 2, 2021
@jogold
Copy link
Contributor

jogold commented May 3, 2021

I think we should use nodejs 12.x

Why not nodejs 14.x which is the current LTS?

@eladb
Copy link
Contributor

eladb commented May 3, 2021

I am okay with node 12 as well.

jogold added a commit to jogold/aws-cdk that referenced this issue May 5, 2021
@jogold
fix(lambda-nodejs): non-deterministic runtime version
4d253d8 
Use `NODEJS_14_X` as default runtime.

Closes aws#13893
@jogold jogold mentioned this issue May 5, 2021
Merged
@mergify mergify bot closed this as completed in #14538 May 5, 2021
mergify bot pushed a commit that referenced this issue May 5, 2021
@jogold
fix(lambda-nodejs): non-deterministic runtime version (#14538)
527f662 
Use `NODEJS_14_X` as default runtime for `NodejsFunction`.

Closes #13893

BREAKING CHANGE: the default runtime version for `NodejsFunction` is now always `NODEJS_14_X` (previously the version was derived from the local NodeJS runtime and could be either 12.x or 14.x).

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented May 5, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

john-tipper pushed a commit to john-tipper/aws-cdk that referenced this issue May 10, 2021
@jogold @john-tipper
fix(lambda-nodejs): non-deterministic runtime version (aws#14538)
1c4dab7 
Use `NODEJS_14_X` as default runtime for `NodejsFunction`.

Closes aws#13893

BREAKING CHANGE: the default runtime version for `NodejsFunction` is now always `NODEJS_14_X` (previously the version was derived from the local NodeJS runtime and could be either 12.x or 14.x).

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
@jogold @hollanddd
fix(lambda-nodejs): non-deterministic runtime version (aws#14538)
b588504 
Use `NODEJS_14_X` as default runtime for `NodejsFunction`.

Closes aws#13893

BREAKING CHANGE: the default runtime version for `NodejsFunction` is now always `NODEJS_14_X` (previously the version was derived from the local NodeJS runtime and could be either 12.x or 14.x).

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-lambda Related to AWS Lambda @aws-cdk/aws-lambda-nodejs bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(lambda-nodejs): non-deterministic runtime version jogold/aws-cdk
4 participants
@eladb @zxkane @jogold @nija-at