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: allow configuration of SnapStart in properties of AWS Lambda Fu… #11576

Merged
merged 9 commits into from Dec 16, 2022
Merged

feat: allow configuration of SnapStart in properties of AWS Lambda Fu… #11576

merged 9 commits into from Dec 16, 2022

Conversation

debae
Copy link
Contributor

@debae debae commented Dec 2, 2022

Adds the possibility to configure the newly added SnapStart property on an AWS Lambda Function.
More info about this can be found : https://docs.aws.amazon.com/lambda/latest/dg/snapstart.html

Closes: #11572

docs/providers/aws/guide/serverless.yml.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
…nction

Added example in documentation how to enable SnapStart.
Make use of specific alias when SnapStart is enabled.

closes issue #11572
…nction

Simplify snapStart to boolean

closes issue #11572
@debae debae requested a review from medikoo December 7, 2022 12:15
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

@debae great thanks for the update. Can we also have some test that covers this functionality?

lib/plugins/aws/package/compile/functions.js Show resolved Hide resolved
…nction

Added extra tests to validate SnapStart configuration

closes issue #11572
…nction

Added validation to prevent combination of provisioned concurrency and snapstart

closes issue #11572
@debae debae requested a review from medikoo December 8, 2022 06:53
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

@debae great thanks for update. We're getting close!

lib/plugins/aws/deploy-function.js Outdated Show resolved Hide resolved
lib/plugins/aws/deploy-function.js Show resolved Hide resolved
lib/plugins/aws/package/compile/functions.js Outdated Show resolved Hide resolved
lib/plugins/aws/package/compile/functions.js Outdated Show resolved Hide resolved
…nction

Simplify configuration of SnapStart
Make use of runServerless for new testcases

closes issue #11572
@debae debae requested a review from medikoo December 9, 2022 06:31
…nction

Make correction in documentation

closes issue #11572
@debae
Copy link
Contributor Author

debae commented Dec 13, 2022

@medikoo All comments have been tackled

Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

@debae Can you confirm what happens if we first deploy with snapStart: true and then redeploy with this setting removed?

It's really important to confirm on that, to be sure our implementation is solid. Thank you!

@debae
Copy link
Contributor Author

debae commented Dec 15, 2022

@debae Can you confirm what happens if we first deploy with snapStart: true and then redeploy with this setting removed?

It's really important to confirm on that, to be sure our implementation is solid. Thank you!

@medikoo Sorry, overlooked your question. Below you can find that scenario as well, and removing the property acts the same as putting it to false resulting in SnapStart:None

Screenshot 2022-12-15 at 21 27 42

Screenshot 2022-12-15 at 21 27 53

Screenshot 2022-12-15 at 21 28 08

Screenshot 2022-12-15 at 21 29 42

@debae debae requested a review from medikoo December 15, 2022 20:31
@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

Base: 85.63% // Head: 85.64% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (14720fc) compared to base (c79737e).
Patch coverage: 94.73% of modified lines in pull request are covered.

❗ Current head 14720fc differs from pull request most recent head f362827. Consider uploading reports for the commit f362827 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #11576   +/-   ##
=======================================
  Coverage   85.63%   85.64%           
=======================================
  Files         314      315    +1     
  Lines       13133    13148   +15     
=======================================
+ Hits        11246    11260   +14     
- Misses       1887     1888    +1     
Impacted Files Coverage Δ
lib/plugins/aws/provider.js 94.70% <ø> (ø)
lib/plugins/aws/deploy-function.js 96.20% <50.00%> (-0.45%) ⬇️
lib/classes/plugin-manager.js 93.72% <100.00%> (ø)
lib/plugins/aws/invoke-local/index.js 69.87% <100.00%> (ø)
lib/plugins/aws/lib/naming.js 97.43% <100.00%> (+0.02%) ⬆️
lib/plugins/aws/package/compile/functions.js 94.92% <100.00%> (+0.14%) ⬆️
lib/utils/import-esm.js 100.00% <100.00%> (ø)
lib/utils/require-with-import-fallback.js 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

That looks great. Thank you @debae !

@medikoo medikoo merged commit adf11b7 into serverless:main Dec 16, 2022
@charlie-harvey
Copy link

charlie-harvey commented Jan 5, 2023

Do I have to do anything special to make sure my PublishedVersion is the one being executed? Or it does it with the alias? Say for SQS?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Lambda Snapstart
3 participants