-
Notifications
You must be signed in to change notification settings - Fork 17
Upgrade lambda node version from 8 to 12 #60
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Have one suggested change.
When this is ready for merging, can you deploy fully to sandbox and post the base URL here for us to kick the tires on? Thanks!
service: sls-simple-reference
stage: sandbox
region: us-east-1
stack: sls-simple-reference-sandbox
resources: 50
api keys:
None
endpoints:
ANY - https://amx4dqzs4j.execute-api.us-east-1.amazonaws.com/sandbox/base
ANY - https://amx4dqzs4j.execute-api.us-east-1.amazonaws.com/sandbox/base/{proxy+}
ANY - https://amx4dqzs4j.execute-api.us-east-1.amazonaws.com/sandbox/xray
ANY - https://amx4dqzs4j.execute-api.us-east-1.amazonaws.com/sandbox/xray/{proxy+}
ANY - https://amx4dqzs4j.execute-api.us-east-1.amazonaws.com/sandbox/vpc
ANY - https://amx4dqzs4j.execute-api.us-east-1.amazonaws.com/sandbox/vpc/{proxy+}
ANY - https://amx4dqzs4j.execute-api.us-east-1.amazonaws.com/sandbox/canary
ANY - https://amx4dqzs4j.execute-api.us-east-1.amazonaws.com/sandbox/canary/{proxy+}
ANY - https://amx4dqzs4j.execute-api.us-east-1.amazonaws.com/sandbox/layers
ANY - https://amx4dqzs4j.execute-api.us-east-1.amazonaws.com/sandbox/layers/{proxy+}
functions:
base: sls-simple-reference-sandbox-base
xray: sls-simple-reference-sandbox-xray
vpc: sls-simple-reference-sandbox-vpc
canary: sls-simple-reference-sandbox-canary
layers: sls-simple-reference-sandbox-layers
layers:
vendor: arn:aws:lambda:us-east-1:819013376994:layer:sls-simple-reference-sandbox-vendor:41
repeat: arn:aws:lambda:us-east-1:819013376994:layer:sls-simple-reference-sandbox-repeat:44 |
package.json
Outdated
@@ -5,6 +5,9 @@ | |||
"repository": "https://github.com/FormidableLabs/aws-lambda-serverless-reference", | |||
"author": "Ryan Roemer <ryan.roemer@formidable.com>", | |||
"license": "MIT", | |||
"engines": { | |||
"node": ">=10" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(picking up on actual visible code thread) My vote here is remove the engines
until we use something that is actually node10+ and use .nvmrc
as our best practice means of getting folks on the right engine. AWS will still support create + update node8 for a few more months and nothing has changed in our very node8 friendly code...
... but I'm not going to block on this if you have consensus from the backend devs team and feel it's a move we should take here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good, i'll remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Thanks for the output report!
One final comment that we can take or leave...
Fixes #32