Bug: Idempotency DynamoDB _put_record method overwrites static_pk_value

[Tankanow]

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

[heitorlessa]

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]

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]

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]

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]

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]

PR up, copied tests over to make this quicker, will credit as usual, and will double check one more time to be safe.

[heitorlessa]

@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]

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]

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]

This is now released under 2.9.1 version!

[heitorlessa]

@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 [email protected] and we can send you a link for a call.

[Tankanow]

Thanks @heitorlessa. I would love to help! I'll send an email over this morning.