Opened 8 months ago
Last modified 8 months ago
#35276 assigned Cleanup/optimization
Push cache backend checks down to backend classes
Reported by: | Adam Johnson | Owned by: | Almaz |
---|---|---|---|
Component: | Core (System checks) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | bcail | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Currently, the three system checks for caches are individual functions in django.core.checks.caches. But two of them relate only to FileBasedCache
, yet still run regardless of which backends are used. This structure causes some issues:
FileBasedCache
and its dependencies are imported even when not used.- Some waste in
check_cache_location_not_exposed()
when noFileBasedCache
is configured, which resolves some paths before checking against caches. - The code structure is a bit messy with repeated loops and
isinstance(cache, FileBasedCache)
conditions.
I propose restructuring these checks to live within the cache backend classes in django.core.cache.backends.*
, adopting the same pattern used for model and field checks, admin checks, and staticfiles finders. (And template backend checks, as I proposed in #35233.)
This would mean:
- Adding
BaseCache.check()
which just doesreturn []
for now. - Pushing the existing two checks down to a new
FileBasedCache.check()
method. - Dropping the existing code.
- Checking tests cover the checks sufficiently and they pass with the new structure.
- Potentially adding a test to cover a custom cache backend with its own check.
Change History (7)
comment:1 by , 8 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 8 months ago
comment:3 by , 8 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 8 months ago
Cc: | added |
---|
comment:7 by , 8 months ago
Has patch: | set |
---|---|
Needs documentation: | set |
Needs tests: | set |
Patch needs improvement: | set |
Agreed, we recently did the same thing with constraint check (0fb104dda287431f5ab74532e45e8471e22b58c8).