Opened 2 hours ago
Last modified 19 seconds ago
#36739 new Bug
`manage.py check` unexpectedly accesses database when JSONField is used
| Reported by: | Storm Heg | Owned by: | |
|---|---|---|---|
| Component: | Core (System checks) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Storm Heg, charetttes, Tim Graham | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
I suspect this is a regression in https://github.com/django/django/commit/3aba1fced8254435b947467739721ec6b4fb865c, the commit message does not suggest to me that this was intentional.
Issue: if you run manage.py check in a project that uses django.db.models.JSONField, the support_json_field SQLite feature detection is now being run (https://github.com/django/django/blob/d506e4a52847ed4fb80754175d76b6b83ff90929/django/db/backends/sqlite3/features.py#L160) which unexpectedly interacts with the database.
This surfaced in the test suite of a package django-otp-webauthn, where it validates its own system checks behave as expected. The test cases in question are not marked as requiring database access, which is why the test against django's main branch started failing last night: https://github.com/Stormbase/django-otp-webauthn/actions/runs/19415998875/job/55581034014
Raising this here as a potential bug, feel free to close if not a bug (though a mention in 6.1 upgrade considerations would be welcome in that case)
Change History (2)
comment:1 by , 2 hours ago
comment:2 by , 67 seconds ago
| Cc: | added |
|---|
Hello Storm, thank you for your report!
This is effectively a change in behavior. I would say expected though as the commit intentionally made check perform all non-database tagged checks against all databases and JSONField.check chain of registration is not tagged Tags.database.
The question now becomes whether we want all checks that potentially access the database to be tagged with Tag.database. From my understanding the Tag.database tag was meant to convey check that the database is setup properly and never might potentially access the database so in this sense I don't think it would be appropriate.
Another way to see it is that staticfiles checks could perform access control gated I/O to determine if they are properly setup (e.g. has access to local or remote destination) like model checks do against the database and we don't treat them differently.
The fact this was surfaced as a potential problem here is that SimpleTestCase was used to avoid database queries and previously the suite of checks performed by default didn't access the database. The thing is even if Django passes databases=None or doesn't call tests tagged with Tag.database it cannot guarantee that the check it calls actually don't access the database, it just happened to be the case within Django core for a long time.
For these reasons I think we should keep things as they are but in all cases I would likely augment the release notes to describe better that this is expected and running check without --tag database might require database access from now on.
Thoughts?
I've prepared a small django project to reproduce the issue: https://github.com/Stormheg/django-36739-manage-py-check-unexpected-database-access