bug: Usage of transaction.atomic is incorrect when when using non-default database alias #800
Description
Background
In my application, I use a non-default database alias along with a router.
DATABASES = {
'default': {
'ENGINE': '',
},
'db1': {
'ENGINE': 'django.db.backends.mysql',
'NAME': REDACTED,
'USER': REDACTED,
'PASSWORD': REDACTED,
'HOST': REDACTED,
'PORT': REDACTED,
},
'db2': {
'ENGINE': 'django.db.backends.mysql',
'NAME': REDACTED,
'USER': REDACTED,
'PASSWORD': REDACTED,
'HOST': REDACTED,
'PORT': REDACTED,
},
}
DATABASE_ROUTERS = ['db_router.MyRouter']
I have a custom router that decides which database each of my application's app models go to, which is based off this django docs page:
class MyRouter:
APP_DATABASE_MAPPING = {
"admin": "db1",
"auditlog": "db1",
"auth": "db1",
"contenttypes": "db1",
"sessions": "db1",
"silk": "db2", # django-silk
"axes": "db2", # django-axes
}
def db_for_read(self, model: models.Model, **hints):
db = self.APP_DATABASE_MAPPING.get(model._meta.app_label)
if not db:
raise ImproperlyConfigured(f"No database configured for {model._meta.app_label}")
logging.debug(f"Returning DB={db} for {model._meta.object_name} READ")
return db
def db_for_write(self, model: models.Model, **hints):
db = self.APP_DATABASE_MAPPING.get(model._meta.app_label)
if not db:
raise ImproperlyConfigured(f"No database configured for {model._meta.app_label}")
logging.debug(f"Returning DB={db} for {model._meta.object_name} WRITE")
return db
Unfortunately, when I make an API request, this fails in django-silk with the following exception:
Traceback (most recent call last):
File "/service/venv/lib/python3.12/site-packages/django/core/handlers/exception.py", line 55, in inner
response = get_response(request)
^^^^^^^^^^^^^^^^^^^^^
File "/service/venv/lib/python3.12/site-packages/silk/middleware.py", line 96, in __call__
response = self.process_response(request, response)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/service/venv/lib/python3.12/site-packages/silk/middleware.py", line 177, in process_response
self._process_response(request, response)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.12/contextlib.py", line 80, in inner
with self._recreate_cm():
^^^^^^^^^^^^^^^^^^^
File "/service/venv/lib/python3.12/site-packages/django/db/transaction.py", line 198, in __enter__
if not connection.get_autocommit():
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/service/venv/lib/python3.12/site-packages/django/db/backends/base/base.py", line 454, in get_autocommit
self.ensure_connection()
^^^^^^^^^^^^^^^^^^^^^^^^
File "/service/venv/lib/python3.12/site-packages/django/db/backends/dummy/base.py", line 20, in complain
raise ImproperlyConfigured(
^
Exception Type: ImproperlyConfigured at /api/foo
Exception Value: settings.DATABASES is improperly configured. Please supply the ENGINE value. Check settings documentation for more details.
Raised during: ninja.operation._sync_view
Cause
It looks like, specifically, this is caused because djano-silk's middleware runs inside a @transaction.atomic().
In Django's source, this looks like this:
def atomic(using=None, savepoint=True, durable=False):
# Bare decorator: @atomic -- although the first argument is called
# `using`, it's actually the function being decorated.
if callable(using):
return Atomic(DEFAULT_DB_ALIAS, savepoint, durable)(using)
# Decorator: @atomic(...) or context manager: with atomic(...): ...
else:
return Atomic(using, savepoint, durable)
In other words, because django-silk does not provide the using kwarg, it will open the transaction over the DEFAULT_DB_ALIAS (default). In my setup, default is not a valid database (intentionally).
Other Impacts
For individuals that are storing silk's data in the non-default database, as has been suggested in other issues like #525, the current transaction implementation is incorrect.
This is because in setups like @CleitonDeLima's in #525, Django's @transaction.atomic() will create the transaction on the default database, not the silk database where the silk data is actually being written.
I would consider this a bug because this means that silk is running non-transactionally in such a case, so there are risks to data integrity.
How other Jazzband packages handle it
I use other Jazzband packages like django-axes that also write to the database inside a transaction, but these work fine in my setup.
If we look at how django-axes's transactions are started, the call looks like with transaction.atomic(using=router.db_for_write(AccessAttempt)):.
In this case, it makes sure to open the transaction for the database that its models are routed to. This means it won't resolve to default in my case, and will make correct use of transactions.