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

feat: Add support to build automatically npm dependencies #292

Closed
wants to merge 9 commits into from

Conversation

maintux
Copy link

@maintux maintux commented Mar 26, 2022

  1. Add support to build automatically npm dependencies, with docker support.
  2. Change default docker image from lambci to official AWS SAM images

Description

As made for pip requirements, this PR adds support to build Nodejs dependencies automatically with or without docker.

Motivation and Context

This PR simplifies the process needed to build and deploy Nodejs functions and, thanks to docker, allows users to build functions without the need to have the interpreter installed locally

Breaking Changes

No breaking changes introduced

How Has This Been Tested?

  • [ x ] I have tested and validated these changes using one or more of the provided examples/* projects
  • [ x ] I have also tested the real deployment of Python and Nodejs functions with and without dependencies.

…ort. Change default docker image from lambci to offical AWS SAM images
@maintux maintux changed the title add support to build automatically npm dependencies feat: add support to build automatically npm dependencies Mar 26, 2022
@maintux maintux changed the title feat: add support to build automatically npm dependencies feat: Add support to build automatically npm dependencies Mar 26, 2022
Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Looks very good! Just a small addition - please mention this new feature in README somewhere and update code in examples to use it for real (this is going to be a confirmation that this PR does what it supposes). Thank you!

README.md Outdated
}, {
path = "src/nodejs14.x-app1",
npm_requirements = true,
nom_tmp_dir = "/tmp/dir/location"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nom_tmp_dir = "/tmp/dir/location"
npm_tmp_dir = "/tmp/dir/location"

Copy link
Author

Choose a reason for hiding this comment

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

Done!

@maintux maintux requested a review from antonbabenko March 26, 2022 17:58
examples/build-package/main.tf Outdated Show resolved Hide resolved
examples/fixtures/nodejs14.x-app1/index.js Outdated Show resolved Hide resolved
@antonbabenko
Copy link
Member

I have finished it in #293 because I didn't have permission to push to your fork.

@antonbabenko
Copy link
Member

This issue has been resolved in version 2.36.0 🎉

@maintux
Copy link
Author

maintux commented Mar 26, 2022

Top! Thanks :)

@github-actions
Copy link

github-actions bot commented Nov 8, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants