Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(framework-provider-aws): ensure storeSnapshot is idempotent #1419

Merged
merged 3 commits into from
Jun 26, 2023
Merged

fix(framework-provider-aws): ensure storeSnapshot is idempotent #1419

merged 3 commits into from
Jun 26, 2023

Conversation

DKarandikar
Copy link
Contributor

Description

Fix DynamoDB call in storeSnapshot by ensuring it is idempotent
This fixes errors when eventHandlers are accessing entities via Booster.entity and also attempting to cache snapshots

Changes

  • Removed ConditionExpression from dyanmoDB call to ensure idempotency

Checks

  • Project Builds
  • Project passes tests and checks
  • Updated documentation accordingly

@DKarandikar
fix(framework-provider-aws): ensure storeSnapshot is idempotent
6baaf74 
ConditionExpression on the dynamodb put was causing duplicate snapshot caching to fail
This directly contradicts the intent in the comment of this function
@DKarandikar
refactor(framework-provider-aws): remove unnecessary try/catch
644fbed 
ConditionalCheckFailedException was thrown by DynamoDB with ConditionExpression, which is now removed
@what-the-diff
Copy link

what-the-diff bot commented Jun 26, 2023

PR Summary

  • Simplified Events Adapter Code
    Cleaned up the events-adapter.ts file by removing unnecessary error handling, conditions and simplifying the dynamoDB.put() method.
  • Improved Code Structure
    Adjusted the positioning of various declarations and logger statements, resulting in better structure and readability of the code.

@ghost
Copy link

ghost commented Jun 26, 2023 •
edited by ghost
Loading

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@DKarandikar DKarandikar marked this pull request as ready for review June 26, 2023 10:25
@javiertoledo
Copy link
Member

Great catch @DKarandikar. I completely missed removing that conditional the last time we changed this part of the code 😅

@NickSeagull
Copy link
Member

Awesome! Looking good

@NickSeagull
Copy link
Member

/integration sha=644fbed

@github-actions
Copy link
Contributor

⌛ Integration tests are running...

Check their status here 👈

@javiertoledo javiertoledo added bug Something isn't working size: S Tasks that involve two or three lines of code provider:aws Related to the AWS cloud provider and removed size: S Tasks that involve two or three lines of code labels Jun 26, 2023
@github-actions
Copy link
Contributor

✅ Integration tests have finished successfully!

@javiertoledo javiertoledo merged commit 1ac1c3b into boostercloud:main Jun 26, 2023
6 checks passed
@DKarandikar DKarandikar deleted the feature/fix_aws_idempotency branch June 26, 2023 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javiertoledo javiertoledo javiertoledo approved these changes

Assignees

@javiertoledo javiertoledo

@DKarandikar DKarandikar

Labels
bug Something isn't working provider:aws Related to the AWS cloud provider
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DKarandikar @javiertoledo @NickSeagull