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

Avoid "island-like" approach for mixins like audit trail/notifications... #6

Open
sergei-maertens opened this issue Dec 20, 2022 · 0 comments
Labels
Refactor Refactor/rework to future proof again
Milestone

Comments

@sergei-maertens
Copy link
Member

sergei-maertens commented Dec 20, 2022

Should be tackled in notifications-api-common (partially) - but the interaction is definitely relevant.

We now have Audit trail, notification and cache mixins/decorators that all operate on the viewset action serializer (instance). Often these mixins query the involved object and/or construct a fresh serializer instance. Serializing each of these instances of the same serializer class results in bad query performance because it repeats queries that were done already, especially to serialize the response data.

This whole mixin approach needs to be revisited for a better/more granular approach giving more control for re-use of instances that can leverage their own caches, so that performance is maximized without dirty tricks.

Additionally, the @transaction.atomic behaviour should either be configurable or managed in the downstream project - now often mixins create stacks of savepoints/savepoint releases that introduce unnecessary query IO.

@sergei-maertens sergei-maertens added the Refactor Refactor/rework to future proof again label Dec 20, 2022
@sergei-maertens sergei-maertens added this to the 2.0 milestone Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor Refactor/rework to future proof again
Projects
None yet
Development

No branches or pull requests

1 participant