This repository has been archived by the owner on Dec 20, 2023. It is now read-only.
handling of multiple transactions #105
Closed
Conversation
This file contains 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
let's avoid running a transaction inside another one.
unless { savepoint: true } is specified, a transaction
is isolated from the others. NOTE: needs more testing
|
oh nice, I see now the travis build error... I'll see if I can fix it. |
|
ok, if I set useTransactions to false by default, all the test passes. I disabled it when writing my app and forgot about that soon after... Anyway, as I don't yet fully know patio inner working logic, I'm not sure how to fix this. I still think that mixing different queries in the same transaction is conceptually wrong and, as you said, confusing. I'll see if I can find a solution to this issue while allowing to create automatically a transaction when saving or updating more models at once. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Hi, with regard to the issue #104 I've tried this change to put multiple transaction in queue rather than mixing them as it happens now.
I'm not sure if this might impact existing applications, but to be honest I think that mixing transaction is wrong in the first place (and as I've reported it may lead to errors) so I dare to say my approach should be the right one :)
Transactions are queued unless { savepoint: true } is specified. In this case everything should work as expected. I suppose that queuing might have a slight performance hit, but I think it's preferable to an unexpected result. I've thought about making them run in parallel, but I've currently no idea how to achieve that without possibily changing the API. Given the fact that it would also be quite a difficult task and I don't have enough time to devote to it right now, I'll leave it for a better time
I'm not an ORM expert, so feel free to review and improve my code :)
Thanks for your work with patio.