[Django] #32788: Transaction APIs do not consult the DB router to choose DB connection
Django
-------------------------------------+-------------------------------------
Reporter: Aditya N | Owner: nobody
Type: | Status: new
Uncategorized |
Component: Database | Version: 3.2
layer (models, ORM) | Keywords: transaction API
Severity: Normal | atomic DB router
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
From the Django docs, for any ORM query made, the DB alias to use is
determined by the following rules:
- Checks if the `using` keyword is used either as a parameter in the
function call or chained with `QuerySet`.
- Consults the DB routers in order until a match is found.
- Falls back to the default router which always returns `default` as the
alias.
Using the router scheme works perfectly for ORM queries. However, when it
comes to using transaction APIs (like the transaction.atomic construct),
it becomes mandatory to specify the `using` kwarg explicitly in all of its
publicly exposed APIs if a DB other than the `default` alias has to be
chosen.
To illustrate why this is a problem, consider the following scenario:
- A DB router exists such that it directs queries to a specific database
based on a value set in ''thread-local'' storage by a middleware.
- When ORM queries from within the view are fired, they automatically get
forwarded to the right DB **without explicitly citing** the `using`
keyword owing to the router.
- But if the transaction.atomic construct is used, the `using` keyword
would have to be explicitly specified each time. While this might seem
fine, it creates the following problems:
i. For large code bases, it becomes highly unwieldy to make sure that
the `using` keyword has been mentioned in every transaction API call.
Also, if one tries to implement the above scheme in an already existing
code base, it would be impractical to add the `using` keyword in all lines
calling the transaction APIs.
ii. It doesn't gel well with the the routers framework.
A proposed work around could to be to add a separate method named
`db_for_transaction` to the routers framework which would be consulted by
the transaction APIs first, before falling back to using the `default` DB
alias.
If we can finalise on this, I could submit a PR for the same.
--
Ticket URL: <https://code.djangoproject.com/ticket/32788>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Aditya N | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: transaction API | Triage Stage:
atomic DB router | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Uncategorized => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/32788#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Aditya N | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: transaction API | Triage Stage:
atomic DB router | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Aditya N (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/32788#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Aditya N | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: transaction API | Triage Stage:
atomic DB router | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Thank you for your detailed report but
[https://docs.djangoproject.com/en/3.2/internals/contributing/bugs-and-
features/#requesting-features this feature request would likely get more
feedback exposure on the mailing list].
> A proposed work around could to be to add a separate method named
db_for_transaction to the routers framework which would be consulted by
the transaction APIs first, before falling back to using the default DB
alias.
Can you think of places where this `db_for_transaction` hook would differ
in any way from what `db_for_write` returns? That's what Django uses
internally in such instances
1.
https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/base.py#L745-L760
2.
https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/base.py#L950
3.
https://github.com/django/django/blob/0d67481a6664a1e66d875eef59b96ed489060601/django/db/models/deletion.py#L400
I get that your asking for a way to avoid explicitly passing
`atomic(using)` all over the place but I'm having a hard time figuring out
in which cases a `db_for_transaction` hook, which cannot be provided any
contextual details like other router methods do, can take an an advised
decision without relying on global state/thread locals.
Maybe that a better API would be a `transaction.default_database(using:
str)` context manager that abstracts the context state encapsulation?
--
Ticket URL: <https://code.djangoproject.com/ticket/32788#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Aditya N | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Keywords: transaction API | Triage Stage:
atomic DB router | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* type: Bug => New feature
* resolution: => needsinfo
Comment:
As Simon said, you'll reach a wider audience if you write to the
DevelopersMailingList about your ideas. We should reach a consensus on the
mailing list before moving this forward.
--
Ticket URL: <https://code.djangoproject.com/ticket/32788#comment:4>