Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#17305 closed Cleanup/optimization (invalid)

Cache middleware keys too difficult to customize

Reported by: Yeago Owned by:
Component: Core (Cache system) Version:
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Attached is a patch which brings many otherwise-unused "utils.cache" utils under the middleware method as classmethods. This is to provide easy hooks so that people can make their own cache key structures.

In case anyone cares, the problem I ran into was that nginx couldn't digest the default cache keys which that middlware creates as they were needlessly complex.

Attachments (2)

diff (2) (23.1 KB) - added by Yeago 5 years ago.
17305.diff (23.1 KB) - added by Preston Holmes 5 years ago.
adding extension to initial diff for proper formatting

Download all attachments as: .zip

Change History (9)

Changed 5 years ago by Yeago

Attachment: diff (2) added

Changed 5 years ago by Preston Holmes

Attachment: 17305.diff added

adding extension to initial diff for proper formatting

comment:1 Changed 5 years ago by Preston Holmes

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

unused in Django itself doesn't mean that other people aren't making use of those util functions in other code in the form of custom cache middleware - this is a rather drastic and backwards incompatible refactor

What problems would you have had in solving your problem if you had simply chosen to subclass the existing middleware and modify it to suit your needs?

comment:2 Changed 5 years ago by Yeago

I had thought of that and I'm open to a patch that makes things backwards compatible--I didn't know how to approach that given the drasticness as you say. Still, I hope that drastic changes aren't a blocker to giving people useful hooks. Also, I had to update the tests and found them to not be difficult to adapt as the classmethods can still be used separately.

Subclassing the existing middleware wouldn't have been fruitful at all. If you take a look at them, the internals are entrenched by these "utils". It is the organization of these utils that makes these middlewares un-subclassable (usefully anyway).

comment:3 Changed 5 years ago by Yeago

Also I think there's an oversight in the Fetch middleware:

https://code.djangoproject.com/browser/django/trunk/django/middleware/cache.py#L133

Perhaps someone can inform me if I am being ignorant, but the Fetch middleware seems to make no reference to settings.CACHE_MIDDLEWARE_UNAUTHENTICATED_ONLY before creating a connection to cache and checking for the value. The only reason it happens to work in the current setup is because the Update happens to skip creation of a key. So even though I have explicitly set UNUAUTH=True, when I use a key that isn't generated by the above utils, it will result in my auth users getting a cached version.

Began ticket about this #17313

comment:4 Changed 5 years ago by Aymeric Augustin

Triage Stage: UnreviewedDesign decision needed

comment:5 Changed 5 years ago by Yeago

Owner: Yeago deleted

FWIW I abandoned this. Nobody besides me minds how its setup now, my related ticket #17313 was closed with a nonsense explanation, and the more I dig into the CACHE_MIDDLEWARE_ANONYMOUS_ONLY the more I realize that setting is totally misleading to begin with and has absolutely nothing to do with anonymous users.

comment:6 Changed 5 years ago by Aymeric Augustin

Resolution: invalid
Status: newclosed

Closing ticket since the reporter has lost interest and I'm skeptical about the idea. I'd need a more detailed proposal, and analysis of the consequence wrt. backwards compatibility, to validate it.

comment:7 Changed 5 years ago by Yeago

The idea was making the cache keys which django creates digestible to other services but django is such a mess in this regard its just easier to duplicate my cache entirely for the sake of letting something else access it.

Note: See TracTickets for help on using tickets.
Back to Top