Skip to content

[Bugfix] fixed populating unprocessed item#7598

Closed
Yliu12 wants to merge 1 commit intolocalstack:masterfrom
Yliu12:master
Closed

[Bugfix] fixed populating unprocessed item#7598
Yliu12 wants to merge 1 commit intolocalstack:masterfrom
Yliu12:master

Conversation

@Yliu12
Copy link
Contributor

@Yliu12 Yliu12 commented Feb 1, 2023

Issue:

#7597

The fix is simple. unprocessed items should be appended to unprocessed[table_name] not unprocessed_items[table_name]

Tested the fix in my docker container.

in dynamodb batchWriteResponse
@giograno giograno self-requested a review February 17, 2023 21:58
Copy link
Member

@giograno giograno left a comment

Choose a reason for hiding this comment

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

Hi @Yliu12, thanks for reporting the bug and providing a fix!
Your solution should work, I also gave it a try! Would you maybe try to add an integration test? You can have a look at test_dynamodb.py.

@viren-nadkarni viren-nadkarni added the aws:dynamodb Amazon DynamoDB label Mar 14, 2023
@viren-nadkarni viren-nadkarni self-requested a review March 14, 2023 11:06
@viren-nadkarni
Copy link
Member

viren-nadkarni commented Mar 15, 2023

I gave it a try but it's non-trivial to return unprocessed items without using error probability config var and running the test in its own session.

Let's merge this fix for now as semantically it makes sense 👍

@viren-nadkarni
Copy link
Member

Tracking at #7879 due to CI glitch here

@localstack-bot
Copy link
Contributor

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request

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

Labels

aws:dynamodb Amazon DynamoDB

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants