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.

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

update s3 lambdas to node20 #10417

Merged
merged 2 commits into from Mar 8, 2024
Merged

update s3 lambdas to node20 #10417

merged 2 commits into from Mar 8, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Mar 8, 2024

Motivation

It seems the pipeline just failed with some missing dependency for some lambdas used in S3: fast-xml-parser. Not sure what produced this, maybe it's not a default dependency anymore.

Edit: ah, well, they updated it. aws/aws-sdk-js-v3#5869
It seems they released a version with an issue, so this is not blocking anymore. I'll remove the added dependency but updating the lambda can't be bad.

Changes

Added the dependency to the list of install.
I've also spotted that Node14 is not supported anymore in lambda, so I've updated the lambda used to Node 20 + to use ES6.

@bentsku bentsku added aws:s3 Amazon Simple Storage Service aws:lambda AWS Lambda semver: patch Non-breaking changes which can be included in patch releases labels Mar 8, 2024
@bentsku bentsku self-assigned this Mar 8, 2024
Copy link

github-actions bot commented Mar 8, 2024

S3 Image Test Results (AMD64 / ARM64)

  2 files    2 suites   3m 30s ⏱️
393 tests 342 ✅  51 💤 0 ❌
786 runs  684 ✅ 102 💤 0 ❌

Results for commit 216752d.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Mar 8, 2024

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 28m 51s ⏱️ + 2m 28s
2 694 tests +2  2 437 ✅ +1  257 💤 +1  0 ❌ ±0 
2 696 runs  +2  2 437 ✅ +1  259 💤 +1  0 ❌ ±0 

Results for commit 216752d. ± Comparison against base commit 9b8268a.

♻️ This comment has been updated with latest results.

@coveralls
Copy link

coveralls commented Mar 8, 2024

Coverage Status

coverage: 85.652% (-0.001%) from 85.653%
when pulling 216752d on fix-s3-issue-with-lambda
into 9b8268a on master.

@bentsku bentsku changed the title fix missing dependency in node lambda + update to Node20 update s3 lambdas to node20 Mar 8, 2024
Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

Thanks for upgrading the lambdas, we should definitely refrain from using deprecated runtimes in our tests.
Only one question about adding fast-xml-parser, since it should be fixed upstream?

)
temp_folder = create_tmp_folder_lambda(
handler_file,
run_command="npm i @aws-sdk/util-endpoints @aws-sdk/client-s3 @aws-sdk/s3-request-presigner @aws-sdk/middleware-endpoint",
run_command="npm i @aws-sdk/util-endpoints @aws-sdk/client-s3 @aws-sdk/s3-request-presigner @aws-sdk/middleware-endpoint fast-xml-parser",
Copy link
Member

Choose a reason for hiding this comment

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

Should we merge this change if they correct it upstream anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops I forgot to do it, it was a bit late 😓 will push the change

@bentsku bentsku merged commit aa664cb into master Mar 8, 2024
36 checks passed
@bentsku bentsku deleted the fix-s3-issue-with-lambda branch March 8, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:lambda AWS Lambda aws:s3 Amazon Simple Storage Service semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants