GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||
GeezerVison is showing github.com — fix: Stop DynamoDB update_online_store overwriting the item when its read fails by ericdelorefice · Pull Request #7046 · feast-dev/feast Conversation
Contributor
What this PR does / why we need it:Problem
For a A missing item comes back as a response without The Devin review on #5916 pointed at this handler in January. This PR adds a reproduction, tests and the fix. Reproductionmoto's DynamoDB server on localhost with the real clients (aiobotocore for the async path, boto3 for the sync one). Only the read failure is injected: a Through Calling the sync With this change both calls raise ChangeBoth handlers log the error and VerificationFive new tests in The DynamoDB online store unit tests go from 47 passed to 52 passed. Which issue(s) this PR fixes:None filed. The reproduction is above. Checks
Testing Strategy
MiscThe read-modify-write is still not atomic when two updates for the same entity run at once. That is a separate question, so I left it alone here. This PR was written with AI assistance (Claude Code); the tests above were run before opening it.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
|
There was an error while loading. Please reload this page.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@ ## master #7046 +/- ## ========================================== + Coverage 51.79% 51.88% +0.08% ========================================== Files 458 458 Lines 57715 57741 +26 Branches 8483 8487 +4 ========================================== + Hits 29896 29961 +65 + Misses 25793 25750 -43 - Partials 2026 2030 +4
*This pull request uses carry forward flags. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
e244d3c to
ae36aa2
Compare
ae36aa2 to
2beac3b
Compare
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…read fails update_online_store and update_online_store_async read the stored item with get_item before applying list_append / prepend. A ClientError from that read (for example ProvisionedThroughputExceededException once botocore's retries are used up) was swallowed, so the item was treated as missing and put_item replaced it with only the values in the call: the list history and any feature not in the call were lost, and the call returned normally. A missing item is reported by the absence of "Item" in the response, not by an exception, so the handler never covered the not-found case. Log and re-raise, as the update_item branch of the same methods already does. Signed-off-by: Eric Del Orefice <[email protected]>
db6a113 to
38e4423
Compare
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.