-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/add-transaction-box #750
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
Conversation
…nes are required for the transaction support
…feature/add-transaction-box
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I think the transaction helper methods can be simplified a little bit. I added more specific comments about them. Then the bigger idea behind this whole transaction support change was to implement a better Transaction ownership principle. Meaning that the method that starts / initializes the transaction must be the one that cancels or commits it. So the helper methods that take in the session shouldn't call the cancelTransaction. They only take the session as param so that the session can be passed to the BasicService so that the db operations are all under the same transaction. But the parent method is responsible for that transaction lifecycle.
And maybe consider matching naming for the transaction helpers. So initialize-, abort-, commitTransaction or Session. A bit nitpicky but might be easier to follow if all helpers are called somethingSession or somethingTransaction.
PlayJeri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done. Good work
Brief description
This PR adds transaction supports for Box module and apply the changes for some classes using transaction methods.
Issue: #742
Change list
openedSession: prevents problems. Basically, a function creates a session then it owns the transaction lifecycle.try/catch/finally: catches errors when working with MongoDB and return them easily.Boxmodule)TransactionCommitError.ts,Transaction.ts.