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.

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

Implementing ACID Transactions #52

Merged
merged 3 commits into from
Dec 28, 2023
Merged

Conversation

yceruto
Copy link
Contributor

@yceruto yceruto commented Dec 27, 2023

Closes #46

Introduces ACID (Atomicity, Consistency, Isolation, Durability) transactions, which are designed to improve data integrity and consistency. This enhancement involves the implementation of a new transaction management system using Doctrine Transaction Middleware within the Messenger component for command handlers.

This approach guarantees that once transactions are committed, they remain stable and unaffected by system failures.

I didn't check the tests yet...

chalasr
chalasr previously approved these changes Dec 27, 2023
Copy link
Collaborator

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Fine by me then. Thanks again!
Looks like we have to update our training material again @mtalrd :)

@chalasr chalasr dismissed their stale review December 27, 2023 16:46

Some tests seem broken with this change

@yceruto yceruto force-pushed the acid_transactions branch 5 times, most recently from 8178d72 to 01456c6 Compare December 27, 2023 16:59
@yceruto
Copy link
Contributor Author

yceruto commented Dec 27, 2023

Some tests seem broken with this change

Yeah, still checking DoctrineBookRepositoryTest...

@yceruto yceruto force-pushed the acid_transactions branch 3 times, most recently from 44a641a to 5cfab63 Compare December 27, 2023 17:53
@yceruto

This comment was marked as outdated.

@yceruto
Copy link
Contributor Author

yceruto commented Dec 27, 2023

I added a NullMiddleware instead of conditionally adding the Doctrine transaction middleware. I preferred this approach because the messenger configuration is clearer. It's ready on my side.

Copy link
Owner

@mtarld mtarld left a comment

Choose a reason for hiding this comment

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

LGTM then! Thanks @yceruto 🙂

@mtarld mtarld merged commit d8e5d9f into mtarld:main Dec 28, 2023
5 checks passed
@yceruto yceruto deleted the acid_transactions branch December 28, 2023 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collection operations not ACID complient
4 participants