Opened 10 months ago

Last modified 3 weeks ago

#35276 assigned Cleanup/optimization

Push cache backend checks down to backend classes

Reported by: Adam Johnson Owned by: tainarapalmeira
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:

  1. FileBasedCache and its dependencies are imported even when not used.
  2. Some waste in check_cache_location_not_exposed() when no FileBasedCache is configured, which resolves some paths before checking against caches.
  3. 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:

  1. Adding BaseCache.check() which just does return [] for now.
  2. Pushing the existing two checks down to a new FileBasedCache.check() method.
  3. Dropping the existing code.
  4. Checking tests cover the checks sufficiently and they pass with the new structure.
  5. Potentially adding a test to cover a custom cache backend with its own check.

Change History (8)

comment:1 by Simon Charette, 10 months ago

Triage Stage: UnreviewedAccepted

comment:2 by Mariusz Felisiak, 10 months ago

Agreed, we recently did the same thing with constraint check (0fb104dda287431f5ab74532e45e8471e22b58c8).

comment:3 by Almaz, 10 months ago

Owner: changed from nobody to Almaz
Status: newassigned

comment:4 by bcail, 9 months ago

Cc: bcail added

comment:5 by bcail, 9 months ago

@Almaz, are you still working on this ticket?

comment:6 by Almaz Amanzholuly, 9 months ago

Yes, I am still working on this ticket

comment:7 by Mariusz Felisiak, 9 months ago

Has patch: set
Needs documentation: set
Needs tests: set
Patch needs improvement: set

comment:8 by tainarapalmeira, 3 weeks ago

Owner: changed from Almaz to tainarapalmeira
Note: See TracTickets for help on using tickets.
Back to Top