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

Add samples for using hot reloading with terraform, fix role arns #210

Merged
merged 6 commits into from
Mar 9, 2023

Conversation

dfangl
Copy link
Contributor

@dfangl dfangl commented Mar 7, 2023

Simple terraform + nodejs lambda functions to demonstrate hot-reloading.

Demonstrates

  • Hot reloading of function code using terraform to deploy the lambda function
  • Hot reloading of function + layer code using terraform to deploy function + layer

Other changes

  • Replace __local__ with hot-reload in all samples, as it is the new default name.
  • Replace simple words used as role arns (like r1) in the samples with complete role arns. Using non-arns as role arns was supported for the old provider, but will not work in the new one, and we should not encourage samples with wrong default values.

Copy link
Contributor

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

Mostly LGTM already 👍

I added a few nits concerning the texts, but nothing is blocking a merge in this case.

It would be great if we could mention which runtimes this should support besides nodejs16.x 🤔

s3_key = "${abspath(path.root)}/lambda_src"
layers = [aws_lambda_layer_version.test_layer.arn]

runtime = "nodejs16.x"
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this wouldn't work with nodejs18.x out of the box right now?

Copy link
Contributor Author

@dfangl dfangl Mar 8, 2023

Choose a reason for hiding this comment

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

Probably not, due to the handler code. Not sure if ES module syntax is required for node18.

@dfangl dfangl merged commit 17b57ea into master Mar 9, 2023
@dfangl dfangl deleted the hot-reload-terraform branch March 9, 2023 09:33
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