Skip to content

fix: disallow nesting of part_of_a_transaction#152

Closed
Giggitycountless wants to merge 1 commit into
kraken-tech:mainfrom
Giggitycountless:feat/disallow-nesting-part-of-transaction
Closed

fix: disallow nesting of part_of_a_transaction#152
Giggitycountless wants to merge 1 commit into
kraken-tech:mainfrom
Giggitycountless:feat/disallow-nesting-part-of-transaction

Conversation

@Giggitycountless

Copy link
Copy Markdown
Contributor

What changed

Add to the atomic call inside part_of_a_transaction so 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=True causes Django to raise a RuntimeError if part_of_a_transaction is called inside an existing atomic block, preventing accidental misuse.

Changes

  1. src/django_subatomic/test.py: Add durable=True to transaction.atomic() call
  2. tests/test_db.py: Add TestPartOfATransaction class with 3 tests:
    • Nesting part_of_a_transaction inside another raises RuntimeError
    • Nesting part_of_a_transaction inside an existing atomic block raises RuntimeError
    • part_of_a_transaction correctly creates a transaction when used standalone
  3. CHANGELOG.md: Document the change in the Unreleased section

How to test

python -m pytest tests/test_db.py::TestPartOfATransaction -v

All 3 new tests pass, and the full test suite (86 passed, 2 skipped) is green.

Fixes #150

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 meshy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

@meshy

meshy commented May 1, 2026

Copy link
Copy Markdown
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).

Comment thread tests/test_db.py
Comment thread tests/test_db.py
Comment thread tests/test_db.py
Comment thread tests/test_db.py
Comment thread tests/test_db.py
@meshy

meshy commented May 1, 2026

Copy link
Copy Markdown
Collaborator

Thanks again @Giggitycountless. This has now been merged as part of #153

@meshy meshy closed this May 1, 2026
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.

Disallow nesting part_of_a_transaction

2 participants