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

fix(lambda): update default runtimes and tests to node 16 everywhere #26921

Merged
merged 25 commits into from
Aug 30, 2023
Merged

fix(lambda): update default runtimes and tests to node 16 everywhere #26921

merged 25 commits into from
Aug 30, 2023

Conversation

mrgrain
Copy link
Contributor

@mrgrain mrgrain commented Aug 29, 2023 •
edited
Loading

Upgrades a few runtime versions from NODEJS_14_X (which is EOL and will enter Deprecation Phase 1 on Nov 27, 2023) to NODEJS_16_X.

Also updates all the tests.

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

@mrgrain mrgrain force-pushed the mrgrain/fix/no-node-14 branch from c32ec99 to c8799ed Compare August 29, 2023 09:36
@github-actions github-actions bot added the p2 label Aug 29, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team August 29, 2023 09:37
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Aug 29, 2023
mrgrain
mrgrain commented Aug 29, 2023
View reviewed changes
packages/aws-cdk-lib/aws-iam/lib/oidc-provider/external.ts
Comment on lines -2 to -8
// the X509 certificate API is available only in node16.
// since we compile the repo against node 14, typechecking it will fail.
// its currently too complex to configure node16 only on this
// file (jsii doesn't support custom tsconfig)
// so we disable typechecking. don't worry, we have sufficient integ tests that
// validate this code doesn't break.
// @ts-nocheck
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New jsii version works with node16

mrgrain
mrgrain commented Aug 29, 2023
View reviewed changes
packages/aws-cdk-lib/aws-lambda-nodejs/lib/bundling.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has non test changes

mrgrain
mrgrain commented Aug 29, 2023
View reviewed changes
packages/aws-cdk-lib/aws-iam/lib/oidc-provider/external.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has non test changes

mrgrain
mrgrain commented Aug 29, 2023
View reviewed changes
packages/aws-cdk-lib/aws-events-targets/lib/aws-api.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has non test changes

mrgrain
mrgrain commented Aug 29, 2023
View reviewed changes
packages/aws-cdk-lib/aws-eks/lib/cluster.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has non test changes

mrgrain
mrgrain commented Aug 29, 2023
View reviewed changes
packages/aws-cdk-lib/aws-ses/lib/receipt-rule.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has non test changes

mrgrain
mrgrain commented Aug 29, 2023
View reviewed changes
packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/evaluate-expression.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has non test changes

@mrgrain
Copy link
Contributor Author

mrgrain commented Aug 29, 2023

SDKv3 todo:

https://github.com/aws/aws-cdk/blob/a523e9fc81021897285b656c49c1e71d888a38f7/packages/aws-cdk-lib/aws-events-targets/lib/aws-api-handler/index.ts

@mrgrain mrgrain force-pushed the mrgrain/fix/no-node-14 branch from c8799ed to e25dc92 Compare August 29, 2023 10:14
mrgrain
mrgrain commented Aug 29, 2023
View reviewed changes
packages/aws-cdk-lib/aws-events-targets/lib/aws-api.ts
@@ -86,7 +86,7 @@ export class AwsApi implements events.IRuleTarget {
code: lambda.Code.fromAsset(path.join(__dirname, 'aws-api-handler'), {
exclude: ['*.ts'],
}),
runtime: lambda.Runtime.NODEJS_14_X,
runtime: lambda.Runtime.NODEJS_16_X,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only to node16 because it is still using SDKv2

@mrgrain mrgrain changed the title chore(lambda): update default runtimes and tests to node 16 fix(lambda): update default runtimes and tests to node 16 everywhere Aug 29, 2023
aws-cdk-automation
aws-cdk-automation previously requested changes Aug 29, 2023
View reviewed changes
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@aws-cdk-automation aws-cdk-automation dismissed their stale review August 29, 2023 12:19

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@mrgrain
dry-run, does not deploy due to missing prerequisites
cef6511 
Attempted a fix, but that deployment takes ages.
@mrgrain
dry-run, stack deploys but assertions fail due to bug in sdk assertio…
a037896 
…ns provider. S3 body is not treated as a buffer
@mrgrain mrgrain temporarily deployed to test-pipeline August 30, 2023 12:03 — with GitHub Actions Inactive
@mrgrain mrgrain marked this pull request as ready for review August 30, 2023 12:04
@aws-cdk-automation
Copy link
Collaborator

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: a037896
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Aug 30, 2023

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit bdce16c into main Aug 30, 2023
@mergify mergify bot deleted the mrgrain/fix/no-node-14 branch August 30, 2023 12:27
@github-actions github-actions bot mentioned this pull request Sep 1, 2023
Closed
@cm-dyoshikawa cm-dyoshikawa mentioned this pull request Sep 1, 2023
Closed
@pahud pahud mentioned this pull request Sep 5, 2023
Closed
2 tasks
@awsdataarchitect awsdataarchitect mentioned this pull request May 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rix0rrr rix0rrr rix0rrr approved these changes

@aws-cdk-automation aws-cdk-automation aws-cdk-automation left review comments

Assignees
No one assigned
Labels
contribution/core This is a PR that came from AWS. p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mrgrain @aws-cdk-automation @rix0rrr