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

nodejs20.x - migrate sample from aws-sdk v2 to v3 #10

Merged
merged 6 commits into from
May 22, 2024
Merged

Conversation

sannya-singal
Copy link
Collaborator

@sannya-singal sannya-singal commented May 21, 2024

Motivation

The current transcription sample is not compatible with the latest node version.

Changes

This PR:

  1. Adds support to the sample to run against node20.x
  2. Migrates aws-sdk v2 to v3
  3. The project now uses ESM module systems
  4. Update the node version used in github workflows pipeline

@sannya-singal sannya-singal self-assigned this May 21, 2024
@sannya-singal sannya-singal requested a review from HarshCasper May 21, 2024 12:22
@sannya-singal sannya-singal marked this pull request as ready for review May 21, 2024 13:21
@sannya-singal sannya-singal requested a review from steffyP May 21, 2024 13:21
Copy link

@steffyP steffyP left a comment

Choose a reason for hiding this comment

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

Great work, @sannya-singal 🙌 🚀 Nice to see the sample working with Nodejs 20 🎉

I only have two questions:

  1. when running the action, there are still some warnings, e.g.
Please update the following actions to use Node.js 20: actions/checkout@v3, actions/setup-python@v4. For more information see: https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/.

I think we should use actions/checkout@v4 and actions/setup-python@v5 .
Could you test this, and see if there are still warnings? 🙏

  1. Not related to this PR, just something I stumbled across: There seem to be some changes in your private repo (where this one was forked from), that is not in the localstack-sample yet. Could you create a PR if the change is relevant?

@sannya-singal
Copy link
Collaborator Author

Thanks @steffyP for the review 🙌 🙇‍♀️

I have removed all the soon to be deprecated github actions in the repository and updated the ones giving warnings, also changed the serverless version to use 3.38.0 as there was a recent release yesterday which is causing some failures here: https://github.com/localstack-samples/sample-serverless-transcribe/actions/runs/9185966236.

Regarding the forked repo diff, its already included in here:

endpoint: 'https://s3.localhost.localstack.cloud:4566',

@sannya-singal sannya-singal merged commit 6d8ff59 into main May 22, 2024
1 check passed
@sannya-singal sannya-singal deleted the node20x branch May 22, 2024 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants