Suor / django-cacheops

A slick ORM cache with automatic granular event-driven invalidation.
BSD 3-Clause "New" or "Revised" License
2.09k stars 227 forks source link

Issue with signals (ManagerMixin._post_save() missing 1 required positional argument: 'using') #466

Closed rez0n closed 9 months ago

rez0n commented 10 months ago

Hi @Suor There are seems some incompatibility issue with (not sure what).

With enabled cacheops app, calls to signals post_save.send(Model, instance=obj) fails with error ManagerMixin._post_save() missing 1 required positional argument: 'using' post_save.send really have 'using' kwarg, but it isn't required actually. Disabling cacheops (or adding using='default' kwarg to the send() method) fix the issue.

Traceback CleanShot 2023-11-01 at 19 53 02

Any ideas how it can be fixed without modifying post_save.send() calls over the codebase? Thanks.

Update: This particular model is not set to caching.

CACHEOPS = {
    'user.user': {'ops': 'get', 'timeout': 60 * 15},
    'user.person': {'ops': 'get', 'timeout': 60 * 15},
    'auth.permission': {'ops': 'all', 'timeout': 60 * 15},
    '*.*': {'timeout': 60 * 15},
}

Update2: Removing wildcard allowance for manual caching '*.*': {'timeout': 60 * 15}, from the settings fix the issue. But it is not possible to enable caching for a model for which I have manual signal senders.

Suor commented 10 months ago

You manually send post_save signal but don't pass all it's params. I don't think this is good behavior. Sending this internal signals yourself might be dangerous at all.

rez0n commented 10 months ago

The problem I raising that these params are optional for sender, but cacheops (actually seems it's dependency funcy) forcing params to be mandatory, which is unexpected behaviour.

Suor commented 10 months ago

I don't think there is such thing as optional parameters to Django's built in signals. Any receiver may count on all documented params to be there. Nobody but Django itself is supposed to be sending them anyway.

If you do you are already on a dangerous territory and you are responsible to make your code to behave exactly like Django does.

Sure this might be fixed in cacheops but then any other signal handler might be broken the same way. And fixing it in cacheops will require it to make some assumptions, i.e. if using is not passed what does it mean? This is not a right place to resolve such questions.