ModelAdmin uses incorrect database in change and delete views

[Jake Howard]

In ModelAdmin.delete_view and ModelAdmin.changelist_view, the transaction is hardcoded to use db_for_write, even if the request is read-only (eg GET). This potentially results in the wrong database being selected (or errors if db_for_write is used to prevent writing to models).

I propose modifying these calls to use db_for_read if the request is read-only (request.method in ("GET", "HEAD", "OPTIONS", "TRACE")). Most users won't notice the difference, as the same database will be selected. But those who are using a custom router will get the database they expect.

[Simon Charette]

I wonder if this is intentional to avoid any form of misrepresentation of data on reads due to factors like replication lag. Given the action is meant to be performed against the write database I could see it being the case.

[Simon Charette]

9459ec82aa12cad9b859c54c2f33f50bec057f2e was added to resolve #26170 through ​!7143 where the point of data inconsistencies was not brought up. I think it's fair that using db_for_read for non-mutable HTTP requests was missed rather than intentional.

[Jake Howard]

I suspect we don't want to create a transaction in the first place if we're using db_for_read?

Perhaps not, but keeping the transaction is less likely to break any existing behaviour, not to mention making more sure the view is internally consistent. Changing the database is going to be less impactful than removing a transaction.

I worry about admin extensions that might create access records against the write database and might start failing here.

Django's built-in LogEntry only logs writes ("Addition", "Change", "Deletion"), so simply viewing a model shouldn't trigger any writes. I could add a test for this, but not sure whether that's overkill.

[Jake Howard]

​PR

[Clinton Christian]

-

[Sarah Boyce <42296566+sarahboyce@…>]

In 53e674d:

Fixed #35520 -- Avoided opening transaction for read-only ModelAdmin requests.