-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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.
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
EOL node engine 6.10 of lambda function #9341
Conversation
nodejs6.10 was EOL on April 30, 2019 ref: https://aws.amazon.com/blogs/developer/node-js-6-is-approaching-end-of-life-upgrade-your-aws-lambda-functions-to-the-node-js-10-lts/ cmd: git grep nodejs6.10 | cut -d ':' -f1 | xargs sed -i.bak -e 's/nodejs6.10/nodejs8.10/g'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @chaspy π Thank you for contributing these fixes. Please see the two items below and reach out if you have any questions or do not have time to implement the changes.
@@ -1724,7 +1724,7 @@ resource "aws_lambda_function" "lambda_function_test" { | |||
function_name = "%s" | |||
role = "${aws_iam_role.iam_for_lambda.arn}" | |||
handler = "exports.example" | |||
runtime = "nodejs6.10" | |||
runtime = "nodejs8.10" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TestAccAWSLambdaFunction_updateRuntime
test that references this test configuration should be performing an update of the runtime
argument and this change will make the runtime value the same as the first test step. Can you please update this to another runtime value (e.g. nodejs10.x
) and ensure the test check matches the new value? Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed: ccfd957
@@ -345,7 +345,7 @@ resource "aws_lambda_layer_version" "lambda_layer_test" { | |||
filename = "test-fixtures/lambdatest.zip" | |||
layer_name = "%s" | |||
|
|||
compatible_runtimes = ["nodejs8.10", "nodejs6.10"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TestAccAWSLambdaLayerVersion_compatibleRuntimes
test that references this test configuration is currently expecting two values for the compatible_runtimes
argument:
Can you please add back a second runtime, e.g. nodejs10.x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed: 95128f1
1f62607
to
95128f1
Compare
Output from acceptance testing:
|
@bflad thank you for your review! Ready for review again π |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks, @chaspy! π
--- PASS: TestAccAWSLambdaFunction_updateRuntime (79.24s)
--- PASS: TestAccAWSLambdaLayerVersion_compatibleRuntimes (19.91s)
--- PASS: TestAccAWSPinpointApp_CampaignHookLambda (30.43s)
I'm going to lock this issue because it has been closed for 30 days β³. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Community Note
Fixes
#0000Seems that the issue has not been reportedI faced the following error: (refer: https://www.terraform.io/docs/providers/aws/r/lambda_permission.html#example-usage)
Unfortunately, nodejs6.10 was EOL on April 30, 2019
ref: https://aws.amazon.com/blogs/developer/node-js-6-is-approaching-end-of-life-upgrade-your-aws-lambda-functions-to-the-node-js-10-lts/
Release note for CHANGELOG:
None of FEATURES, ENHANCEMENT and BUG FIX. This is just updating doc and test.
Output from acceptance testing: