I’ve been working on a spec for months. Finally…every stakeholder approves. I’ve cataloged every piece of code that needs to be migrated in a Notion DB. I’m working on one of the first PRs and then I see it…
db
.
commit()
“…ffffuuuuuummmmccckkkk mmmmmmeee…”
The fucking “transactions” are being committed while wearing a coat of atomicity.
“fuck, fuck, fuck, how the fuck did I miss this.”
Looking through my notes…I see…“investigate random-ass commits” and I forgot to account for them…
ORMs can blow your leg off, this ain’t that rant.
Parametrized SQL statements can be your saviour, this ain’t that rant.
Query builders can be a nice middle ground, this isn’t that rant.
This rant is about fucking up code organization and abstractions so badly that code wrapped inside transactions ain’t atomic.
Sample Exhibits
#
These are meant to be illustrative, so code has been cleaned up and doesn’t contain all the tiny details of any framework/library. Sessions being created, etc.
The Hidden Enemy
#
class
DBAccess
:
@staticmethod
def
create_records
(records: List[DomainModel]):
# All records are meant to be a single transaction
with
transaction():
for
r
in
records:
DBAccess
.
create_main_records(r)
# calls helpers that eventually call commit()
# Another transaction is auto-started
DBAccess
.
create_details_records(r)
DBAccess
.
create_records(recs)
The code is fucking around with manual commits 2+ levels away from the transaction decorator/context manager. So, when
create_main_records
is called from the manager, no one has ANY god damn idea that the method commits.
The Silent Frenemy
#
class
DBAccess
:
@staticmethod
def
fetch_records
(ids: List[int])
->
List[DBModel]:
db_models
=
session
.
query(DBModel)
.
filter(DBModel
.
id
.
in_(ids))
.
all()
return
cast(List[DBModel], db_models)
with
transaction():
db_models
=
DBAccess
.
fetch_records(ids)
db_models[
0
]
.
yo_mama_fat
=
True
# This is a silent DB write
# context manager exit saves your ass
The code is passing DB models around, treating them like normal domain models, setting properties that seem to do nothing other than setting the value but underneath it all, there are DB writes being triggered.
Daddy’s Milk Run
#
class
DBAccess
:
@staticmethod
def
fetch_records
(ids: List[int])
->
List[DBModel]:
db_models
=
session
.
query(DBModel)
.
filter(DBModel
.
id
.
in_(ids))
.
all()
return
cast(List[DBModel], db_models)
# transaction has been commented out
# with transaction():
db_models
=
DBAccess
.
fetch_records(ids)
db_models[
0
]
.
yo_mama_fat
=
True
# request ends, data poofs into the ether
# like the programmer’s dad leaving to get milk and never returning
The code doesn’t auto-commit and has no transactions, it’s causing data loss.
Want to hear more of my unhinged rants? Drop your email.
Subscribe
or use the
RSS feed
if you hate email.
Who is responsible?
#
DO NOT blame the framework
DO NOT blame the business requirement.
The programmer is the only one responsible. In a lot of cases, you can be absolved of the blame for the state of the code, but NOT in this case. ANYTHING is better than random ass commits.
In the end, your ass, whether you wrote the code or not (my case) is on the line for fixing this shit. Like I have been for months.
WTF are we learning?
#
DO NOT sprinkle in DB sessions or transactions like salt-bae. If you do so,
expect to crash and burn like him
. The DB abstraction layer owns the transactions and commits.
DO NOT pass DB models in and out of the DB layer.
Read 1+2 again, then read them again. Absorb them.
DO NOT fuck with transactions, commits, queries or whatever outside of the DB layer.
Don’t even look at it, don’t even think about it
.
DO NOT commit manually. Especially if you use context managers or decorators.
You are not that guy/gal pal
.
DO NOT split code across helpers.
Atomic multi-writes = one function
. Write the 3 line god-damn insert again. The duplication keeps atomicity visible and you won’t create hidden enemies. Otherwise, watch your god-damn back.
How do you enforce the lessons?
#
AST Analysis:
#
Either through custom tests that use
AST
analysis or through
flake8
/linters. Whatever you choose, it should:
Ban commits from being called manually,
COMPLETELY
Ban db session from being accessed outside the DB access layer
Ban transactions from being accessed outside the DB access layer
Ban DB model imports from outside the DB access layer
Bans using AST
#
class
TestDBBoundaries
:
def
test_no_manual_commits
(self):
violations
=
[]
for
path, tree
in
parsed_source_files():
# ast.parse over your source tree
for
node
in
ast
.
walk(tree):
if
not
isinstance(node, ast
.
Call):
continue
func
=
node
.
func
if
(
(isinstance(func, ast
.
Attribute)
and
func
.
attr
==
“commit”
)
# session.commit()
or
(isinstance(func, ast
.
Name)
and
func
.
id
==
“commit”
)
# commit = session.commit; commit()
):
violations
.
append(
f
”
{
path
}
:
{
node
.
lineno
}
manual commit()”
)
assert
not
violations,
”
\n
”
.
join(violations)
Bans using flake8
#
Same walk, wearing flake8’s trench coat:
import
ast
class
BanManualCommits
:
def
__init__
(self, tree):
self
.
tree
=
tree
def
run
(self):
for
node
in
ast
.
walk(self
.
tree):
if
not
isinstance(node, ast
.
Call):
continue
func
=
node
.
func
if
(
(isinstance(func, ast
.
Attribute)
and
func
.
attr
==
“commit”
)
or
(isinstance(func, ast
.
Name)
and
func
.
id
==
“commit”
)
):
yield
node
.
lineno, node
.
col_offset,
“DB001 manual commit()”
, type(self)
What the registration looks like:
[
project
.
entry-points
.
“flake8.extension”
]
DB0
=
“flake8_db_boundaries:BanManualCommits”
AST or flake8?
#
Use AST tests when:
the rule needs to look at the whole codebase
you can’t touch the shared linter config cause that’ll (rightfully) callout other teams’ shitty code and egos are too fragile
Otherwise stick with flake8/linters. They are the standard tools for a reason.
Combined with LLM Support
#
LLMs should be used to:
Ban any DB models from being returned by the DB layer
AST can’t catch this one and neither can mypy, cause:
return annotations lie
cast()
launders types (The Silent Frenemy)
a codebase with random-ass commits doesn’t inspire confidence in the typing
So this check is an LLM pass in the CI/CD pipeline. A deterministic script dumps the DB access layer’s public functions/methods, then the LLM answers a single question: “does anything return a DB model instead of a domain model?” No agentic drama, create a prompt and throw the output from the script in there. Results:
yes ->
human review
maybe -> human review
no -> pass Go, collect your paycheque
✌️
#
If you find me insufferable, fine. The
one
take away:
The DB abstraction layer owns the commits and transactions. Everything else is a workaround for getting that wrong.
So, you don’t have to spend MONTHS refactoring the whole god-damn codebase to fix fucked up transactions after discovering a random-ass commit.
ALSO READ THIS GOD-DAMN BOOK:
Domain-Driven Design: Tackling Complexity in the Heart of Software
.
Want to hear more of my unhinged rants? Drop your email.
Subscribe
or use the
RSS feed
if you hate email.
