-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Fix provisioned concurrency set on Lambda alias #12592
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
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.
Thank you for the fix @dfangl 👏👏👏
Good catch 🕵️
@@ -156,6 +156,12 @@ def get_invocation_lease( | |||
provisioned_concurrency_config = function.provisioned_concurrency_configs.get( | |||
function_version.id.qualifier | |||
) |
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.
docs: Would a docstring help to clarify our findings?
# Look up a potential provisioned concurrency config for aliases because a function version shares provisioned concurrency with aliases. However, the config only applies to a single function version or alias on the CRUD level.
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.
I added some comments on 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.
LGTM 👍
localstack-core/localstack/services/lambda_/invocation/counting_service.py
Show resolved
Hide resolved
@@ -2539,6 +2539,59 @@ def test_provisioned_concurrency(self, create_lambda_function, snapshot, aws_cli | |||
result2 = json.load(invoke_result2["Payload"]) | |||
assert result2 == "on-demand" | |||
|
|||
@markers.aws.validated | |||
def test_provisioned_concurrency_on_alias(self, create_lambda_function, snapshot, aws_client): |
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.
question: Should we be confirming within the test that no additional containers are brought up?
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.
There will be an additional container brought up for the last invocation, but in general, confirming that we hit the right invocation type environment should be sufficient, otherwise we cannot validate it against AWS as well.
Motivation
Currently, when setting provisioned concurrency on a lambda alias pointing to a version, and then invoking it, there will still be another container spawned up on demand.
This happens due to an error in the lookup of the provisioned concurrency config in the counting service, which issues leases for environments.
Changes