-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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(cli): support hotswapping Lambda functions that use Docker images #18319
Conversation
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
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.
Looks great @tmokmss! I tested the functionality, and it works beautifully. If we could only separate the inline code changes from the Docker image changes, that would be great.
packages/aws-cdk/test/api/hotswap/lambda-functions-hotswap-deployments.test.ts
Outdated
Show resolved
Hide resolved
One small detail: I see that sometimes the deployment takes a while with the Docker images - I keep refreshing, but the old code is still there for like 5 to 10 seconds. It could be because my test app also uses API Gateway. Did you see that behavior too? Maybe we need to wait for something in the Docker image case? |
Hi @skinny85 thank you for the review! I agree with your point, possibly submitting a new revision and a PR for inline code today. As for a few seconds delay on Lambda code update, I usually see them even in non-Docker Lambda functions. I guess it's a Lambda internal matter and there isn't particularly much to do for us. |
Of course! Thank you for submitting the PR, it's great 🙂.
Really? That's super interesting! Any chance you can record a quick video/GIF showing me that happening? I actually have a sneaking suspicion there might be something we can do there, as I had to do something similar in this code that deals with Lambda Versions. |
@skinny85 Considering this statement below, I guess non-docker and non-VPC Lambda functions are currently updated synchronously without any delay.
BTW I was using hotswap with VPC Lambda, which I guess is why I'm experiencing the delay even in non-Docker functions! But be careful, the blog also says there is a plan that all the functions will going to have equally the delay from February 1 2022. So I guess we should use edit) Calling |
Pull request has been modified.
Thanks for all of the information @tmokmss, that's super, super valuable! I agree with your conclusion: let's only worry about Docker image hotswapping in this PR, and we'll tackle waiting for the update to complete in a separate PR. I'll create an issue to track that work. |
This reverts commit bb4eb4d.
Hi @skinny85, I fixed tests and also opened a PR for InlineCode hotswap. I hope you can check them when you get a chance 🙏 |
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.
Looks fantastic @tmokmss, short and super clean! Thanks for the contribution.
Ah, one last detail - can you update the ReadMe of the package with this new capability? In this chapter: https://github.com/aws/aws-cdk/blob/master/packages/aws-cdk/README.md#hotswap-deployments-for-faster-development |
@skinny85 I'm not very sure what words to add since it already covers Lambda code hotswap capability, maybe like this? - Code asset and tag changes of AWS Lambda functions.
+ Code asset (including Docker Lambda) and tag changes of AWS Lambda functions. or maybe adding |
How about |
Pull request has been modified.
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.
Thanks for the contribution @tmokmss!
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
) Similarly to #18319, this PR supports hotswap of Lambda functions that use `InlineCode`. `InlineCode` uses [CloudFormation `ZipFile` feature](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-lambda-function-code.html#:~:text=requires%3A%20No%20interruption-,ZipFile,-\(Node.js%20and). To emulate its behavior, we create a zip file of the provided inline code with its filename `index.js` or `index.py` according to the runtime (CFn only supports python or nodejs for ZipFile), and pass the file's binary buffer to Lambda API. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@tmokmss Do you have any sample on how to use this? I'm using |
Hi @jeromevdl I confirmed it works with e.g. new lambda.DockerImageFunction(this, 'Handler', {
code: lambda.DockerImageCode.fromImageAsset('lambda', {
}),
}); What |
Thanks for the answer, I'm on CDK v2... Any idea when it will be released? |
Considering the last v2 release was two weeks ago, it should be soon:) (I'm not very sure though as I'm just a contributor |
Are you sure you're just a simple contributor? ;-) It was just released, thank you! |
…#18408) Similarly to aws#18319, this PR supports hotswap of Lambda functions that use `InlineCode`. `InlineCode` uses [CloudFormation `ZipFile` feature](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-lambda-function-code.html#:~:text=requires%3A%20No%20interruption-,ZipFile,-\(Node.js%20and). To emulate its behavior, we create a zip file of the provided inline code with its filename `index.js` or `index.py` according to the runtime (CFn only supports python or nodejs for ZipFile), and pass the file's binary buffer to Lambda API. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
closes #18302
We must just update
ImageUri
with the new ECR image url.PR for
InlineCode
hotswap: #18408----
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license