Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only() in bulk sync breaks django-cacheop's post_delete logic #17

Closed
voron3x opened this issue Aug 17, 2020 · 2 comments
Closed

only() in bulk sync breaks django-cacheop's post_delete logic #17

voron3x opened this issue Aug 17, 2020 · 2 comments

Comments

@voron3x
Copy link

voron3x commented Aug 17, 2020

Hi!

Cacheops have automatic invalidation logic by post_save signal, but with "only" chain method this logic is not working correctly (see this comment Suor/django-cacheops#348 (comment)).

Can i remove this method? And for "only" case we can use managed=False models with subset of fields as recommended in django doc for defer/only methods (https://docs.djangoproject.com/en/3.1/ref/models/querysets/#defer).

voron3x added a commit to voron3x/django-bulk-sync that referenced this issue Aug 17, 2020
@ses4j
Copy link
Contributor

ses4j commented Aug 17, 2020

I don't really understand what cacheops is doing to cause the problem. I don't want to remove the only() unless I can replace it's functionality somehow, it's a worthwhile optimization.

I don't use pre/post signals much (I don't like the paradigm, but I digress...). But since bulk_create and bulk_update don't emit signals, the delete step shouldn't either. For that reason, I should probably replace:

objs.filter(pk__in=[_.pk for _ in list(obj_dict.values())]).delete()

with something like _raw_delete: https://code.djangoproject.com/ticket/9519

That will probaly make cacheops not crash, but also break it since it depends on the hooks. But, it probably was already broken becuase the create and update hooks aren't being invoked.

Thoughts?

@voron3x
Copy link
Author

voron3x commented Aug 18, 2020

Yes you are correct, I need use cacheops.bulk_update for populate cache. And for these maybe best choice would be use bulk_compare with custom logic for bulk update, delete and insert. But due performance reasoning I don`t wont use compare by fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants