Repository navigation
Bug: Idempotency DynamoDB _put_record method overwrites static_pk_value #1968
Description
Expected Behaviour
When I pass a static_pk_value value into the DynamoDBPersistenceLayer, I expect that value to be used in the Primary Key.
Current Behaviour
The PK value is overwritten with data_record.idempotency_key.
Code snippet
[This test](https://github.com/Tankanow/aws-lambda-powertools-python/pull/1/files) shows the problem. Running this test results in this failure:
E aws_lambda_powertools.utilities.idempotency.exceptions.IdempotencyPersistenceLayerError: Failed to save in progress record to idempotency store - (Error getting response stub for operation PutItem: Expected parameters:
E {'ConditionExpression': 'attribute_not_exists(#id) OR #expiry < :now OR '
E '(#status = :inprogress AND '
E 'attribute_exists(#in_progress_expiry) AND '
E '#in_progress_expiry < :now_in_millis)',
E 'ExpressionAttributeNames': {'#expiry': 'expiration',
E '#id': 'id',
E '#in_progress_expiry': 'in_progress_expiration',
E '#status': 'status'},
E 'ExpressionAttributeValues': {':inprogress': {'S': 'INPROGRESS'},
E ':now': {'N': <ANY>},
E ':now_in_millis': {'N': <ANY>}},
E 'Item': {'expiration': {'N': <ANY>},
E 'id': {'S': 'static-value'},
E 'in_progress_expiration': {'N': <ANY>},
E 'sk': {'S': 'test-func.functional.idempotency.test_idempotency.test_idempotent_lambda_compound_static_pk_value_has_correct_pk.<locals>.lambda_handler#be4755f20c4e6f94f43bc3f5849a26fb'},
E 'status': {'S': 'INPROGRESS'}},
E 'TableName': 'TEST_TABLE'},
E but received:
E {'ConditionExpression': 'attribute_not_exists(#id) OR #expiry < :now OR '
E '(#status = :inprogress AND '
E 'attribute_exists(#in_progress_expiry) AND '
E '#in_progress_expiry < :now_in_millis)',
E 'ExpressionAttributeNames': {'#expiry': 'expiration',
E '#id': 'id',
E '#in_progress_expiry': 'in_progress_expiration',
E '#status': 'status'},
E 'ExpressionAttributeValues': {':inprogress': {'S': 'INPROGRESS'},
E ':now': {'N': '1677689099'},
E ':now_in_millis': {'N': '1677689099925'}},
E 'Item': {'expiration': {'N': '1677692699'},
E 'id': {'S': 'test-func.functional.idempotency.test_idempotency.test_idempotent_lambda_compound_static_pk_value_has_correct_pk.<locals>.lambda_handler#be4755f20c4e6f94f43bc3f5849a26fb'},
E 'in_progress_expiration': {'N': '1677689100925'},
E 'sk': {'S': 'test-func.functional.idempotency.test_idempotency.test_idempotent_lambda_compound_static_pk_value_has_correct_pk.<locals>.lambda_handler#be4755f20c4e6f94f43bc3f5849a26fb'},
E 'status': {'S': 'INPROGRESS'}},
E 'TableName': 'TEST_TABLE'})Possible Solution
Simply remove the offending line, self.key_attr: {"S": data_record.idempotency_key}, in https://github.com/Tankanow/aws-lambda-powertools-python/blob/5f9addda12d3c4066fe7f480f99184126b154d57/aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py#L149.
Steps to Reproduce
Run the tests in this PR
I will happily fix if this issue is accepted.
AWS Lambda Powertools for Python version
latest
AWS Lambda function runtime
3.9
Packaging format used
PyPi
Debugging logs
No response
Reacted by Heitor Lessa
Activity
heitorlessa commentedon Mar 1, 2023
Hey @Tankanow - feel free to send a PR for this, as you're a highly regarded contributor ;-)
I do wonder if we missed this earlier during implementation, or if it's a regression.
Since you last contributed we now have E2E tests ;-)
Thank you as always
PS: dinner time now but can look into it first thing tomorrow morning, and cut a patch release
Tankanow commentedon Mar 1, 2023
Hi @heitorlessa, I believe it is a regression. We found it in our unit tests that consume this code after upgrading to
2.8.0.heitorlessa commentedon Mar 1, 2023
Hmmm no bueno and surprises me since we had tests for that.
Last change on Idempotency persistence later was this that made it thread-safe.
Barely keeping my eyes open but can investigate it tomorrow morning (CET) and cut a patch release ASAP.
#1899
heitorlessa commentedon Mar 1, 2023
sadly it was added in the PR that addressed thread-safety (larger structural change) - got my laptop now, eye dropper and looking into it what test was changed that shouldn't have, and cutting a patch release if confirmed.
heitorlessa commentedon Mar 1, 2023
looking at git blame and history, there wasn't an explicit test for this feature
static_pk_value, otherwise that dict mutation line would've raised an error.@Tankanow You opened a PR in your base branch instead of the origin ;) Feel free to correct it, and I can make minor modifications, merge, and release
heitorlessa commentedon Mar 1, 2023
PR up, copied tests over to make this quicker, will credit as usual, and will double check one more time to be safe.
heitorlessa commentedon Mar 1, 2023
@Tankanow - good to go. I'll wait a few minutes to hear it's okay to merge from you, otherwise I'll go ahead.
heitorlessa commentedon Mar 1, 2023
Release in process - might take ~2m for PyPi and 30m for Lambda Layer (v23) to be available across all regions.
https://github.com/awslabs/aws-lambda-powertools-python/actions/runs/4307851934
heitorlessa commentedon Mar 1, 2023
PyPi test is under maintenance... 🤦 removed PyPi test endpoint from the release temporarily, and it's now available as 2.9.1: https://pypi.org/project/aws-lambda-powertools/
Starting to build Layers and will deploy across all regions if canaries pass - ~30m ETA.
Progress: https://github.com/awslabs/aws-lambda-powertools-python/actions/runs/4307953507/jobs/7513645453
github-actions commentedon Mar 1, 2023
This is now released under 2.9.1 version!
heitorlessa commentedon Mar 2, 2023
@Tankanow please let us know if it didn't fix it somehow. Just published the release notes and credited you accordingly
PS: I'd love to hear from you one of these days on how you're using Powertools, share our main ideas for this year (all public either way), and gather your feedback on areas we could innovate/do better -- shoot an email to aws-lambda-powertools-feedback@amazon.com and we can send you a link for a call.
Tankanow commentedon Mar 2, 2023
Thanks @heitorlessa. I would love to help! I'll send an email over this morning.