Ticket 35276 FileBasedCache check method #18008
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello! Thank you for your contribution 💪
As it's your first contribution be sure to check out the patch review checklist.
If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!
If you have any design or process questions then you can ask in the Django forum.
Welcome aboard ⛵️!
Pro100-Almaz
force-pushed
the
ticket_35276
branch
from
3545cd3 to
78a6fec
Compare
Pro100-Almaz
force-pushed
the
ticket_35276
branch
from
43019af to
9870a6d
Compare
| build/ | ||
| tests/report/ | ||
| tests/screenshots/ | ||
| ./idea |
There was a problem hiding this comment.
I don't think this should be added see comment at top.
| if location is not None and not pathlib.Path(location).is_absolute(): | ||
| return [ | ||
| Warning( | ||
| f"Your '{alias_name}' cache LOCATION path is relative. Use an " | ||
| f"absolute path instead.", | ||
| id="caches.W003", | ||
| ) | ||
| return errors | ||
| ] |
There was a problem hiding this comment.
Will this work for multiple file based caches?
There was a problem hiding this comment.
It will not work, made changes. Thank you for your comment!
|
|
||
|
|
||
| @register(Tags.caches) | ||
| def check_file_based_cache_is_absolute(app_configs, **kwargs): |
There was a problem hiding this comment.
Wasn't the point of ticket to drop these here?
There was a problem hiding this comment.
I have removed instance check of cache for FileBaseCache and lightened part with cache iteration. Also I thought to remove this two function (in cache.py) and directly address to main class of FileBaseCache, but I stayed with the opinion to leave them. Could you suggest which way is better?
There was a problem hiding this comment.
@Pro100-Almaz Thanks 👍 I left initial comments. Also, tests updates and docs changes (we should mention it here) are missing.
django/core/cache/backends/base.py
Outdated
There was a problem hiding this comment.
The base implementation should return an empty list instead of raising exceptions.
django/core/checks/caches.py
Outdated
There was a problem hiding this comment.
The main idea is to run check() on all caches, not to leave the same functions that will run class methods. Check what we did in a similar patches, e.g. 0fb104d.
|
Please don't leave the PR description as "Provide a concise overview of the issue or rationale behind the proposed changes." - if you don't know what to write you can always say "see ticket". |
|
Use the PR and commit title: “Fixed #35276 -- Moved cache checks to backend classes.” Squash all your commits into one, at least when you’re done working. You can follow my guide: https://adamj.eu/tech/2022/03/25/how-to-squash-and-rebase-a-git-branch/ . |
Pro100-Almaz
force-pushed
the
ticket_35276
branch
from
63dfe38 to
9870a6d
Compare
There was a problem hiding this comment.
Should be an instance method, you should see when you get the loop in place.
|
Superseded by #19426. |
4 participants
Trac ticket number
ticket-35276
Branch description
check ticket
Checklist
mainbranch.