fix: disallow nesting of part_of_a_transaction#152
Closed
Giggitycountless wants to merge 1 commit into
Closed
Conversation
Add durable=True to the atomic call inside part_of_a_transaction so that nesting is explicitly disallowed. This prevents tests from accidentally simulating nested transaction behaviour, which does not reflect real-world usage. Fixes kraken-tech#150
meshy
approved these changes
May 1, 2026
meshy
left a comment
Collaborator
There was a problem hiding this comment.
Thank you for contributing this! I realised last night that this blocks another ticket I've been working on, so I was going to pick this up this morning, but I see you've beaten me to it!
There are a couple of minor changes I'd like to see to this, but seeing as I was going to work in this this morning, I'm going to merge this now and apply my tweaks on top.
Thanks again!
Collaborator
|
Actually, this has highlighted a minor issue in the CI pipeline. I'm going to address that before getting around to merging this. I may as well list the tweaks I'm going to make on this PR now, so I don't forget (and in case you happen to get to them first). |
meshy
requested changes
May 1, 2026
Collaborator
|
Thanks again @Giggitycountless. This has now been merged as part of #153 |
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
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.
What changed
Add to the
atomiccall insidepart_of_a_transactionso that nesting is explicitly disallowed.Why
As noted in #150, just as it doesn't make sense for transactions to be nested, it doesn't make sense for tests to simulate nesting of parts of transactions. Adding
durable=Truecauses Django to raise aRuntimeErrorifpart_of_a_transactionis called inside an existing atomic block, preventing accidental misuse.Changes
durable=Truetotransaction.atomic()callTestPartOfATransactionclass with 3 tests:part_of_a_transactioninside another raisesRuntimeErrorpart_of_a_transactioninside an existingatomicblock raisesRuntimeErrorpart_of_a_transactioncorrectly creates a transaction when used standaloneHow to test
All 3 new tests pass, and the full test suite (86 passed, 2 skipped) is green.
Fixes #150