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

Feature/transactions #175

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

snava10
Copy link
Contributor

@snava10 snava10 commented Dec 24, 2020

#143

Includes basic support for transactions (updates and find operations to start with).
Implemented on top of H2 transactions, so it is only available for the H2 Backend.
Real mongo test of transactions using a replica set.

@coveralls
Copy link

coveralls commented Dec 24, 2020

Coverage Status

Coverage decreased (-0.2%) to 96.757% when pulling 8c0f227 on snava10:feature/transactions into 13059be on bwaldvogel:master.

@bwaldvogel
Copy link
Owner

@snava10: Thanks for this large contribution. I really appreciate your work!

However, adding H2 as dependency to core and even exposing the H2 classes in the core API is going in the wrong direction.
I think we would need to hide the transaction implementation details behind some session/transaction facade that will then be implemented in the H2 backend. It would throw an UnsupportedOperationException in the other backends or even "silently" ignore start/commit transaction requests if the user explicitly wants it.

However, please also see my comment on why I even think that supporting transactions could be out-of-scope for this project. Please convince me that I’m wrong! :)

@snava10
Copy link
Contributor Author

snava10 commented Apr 13, 2021

@bwaldvogel I agree with you regarding H2 in the core module. This pull request needs some serious refactoring, I lack the time at the moment, I'll come back to it when I can, or maybe someone else wants to contribute

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.

3 participants