Closed
Conversation
Make it a scoped service so only 1 MongoClient is created per request Use that to expose transactions which are now shared across all Repos
hahn-kev
force-pushed
the
expose-mongo-transactions
branch
from
b73f10a to
8704d45
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3369 +/- ##
==========================================
- Coverage 74.73% 74.49% -0.25%
==========================================
Files 280 281 +1
Lines 10743 10801 +58
Branches 1297 1300 +3
==========================================
+ Hits 8029 8046 +17
- Misses 2349 2389 +40
- Partials 365 366 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Collaborator
|
I extended this pr's branch with some cleanup and a fix (adding the missing |
Collaborator
|
Replaced by #3484 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From this discussion and a follow up with Jason he mentioned that transactions haven't been exposed to utilize them in mongo.
In order for transactions to work across repos there needs to only be one Mongo client in a given request, I've refactored all the type specific contexts to get the IMongoDb from the new IMongoDbContext which is a scoped service so there's only one instance per request.
Then I changed a couple methods in the Merge service to make changes inside transactions. You'll notice that the transaction is not aborted in our user code, this is because the transaction is disposable and if it's disposed before it's committed then it will be aborted, so if an exception is thrown between Begin and Commit then the changes made will be rolled back.
Feel free to suggest changes, I don't really love how I had to mock out and expose the transactions via the IMongoDbContext which also contains the IMongoDatabase, but I didn't really feel like making a separate service just for transactions, @jasonleenaylor let me know what you think there if we want to split those out.
This change is