Reduced queries in full clean #143
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
August 31, 2020 14:43
At least, for Binder models this is the case. All models managed by Binder views should inherit from BinderModel, and if not, the user should code validation error checking in _store(). In all our projects we use django-fullclean because being able to accidentally save invalid models is a huge mind fuck. Thus, it is pointless to call full_clean() in our views, as it means we'll be running potentially expensive validations twice. For example, relation fields check that the foreign relation exists, which requires a query per relation field. It is technically a breaking change, but for most projects this will be a non-issue. The test suite of Boekestijn passed except for one test which was specifically for its User view, which uses a non-Binder model. This was relatively easy to convert to use a full_clean() in its _store() method.
This leverages our custom full_clean implementation (#141) in combination with the LoadedValues mixin. This allows us to know when a value did not change and avoid validating the foreign key just before save, because the foreign key was fine as it was stored, so it cannot suddenly become invalid. For complex models with many relations or which are saved a lot, this makes a big difference, as the reduced number of queries can make things quite a bit more efficient.
Codecov Report
@@ Coverage Diff @@
## master #143 +/- ##
==========================================
- Coverage 78.19% 78.09% -0.11%
==========================================
Files 24 24
Lines 3252 3268 +16
==========================================
+ Hits 2543 2552 +9
- Misses 709 716 +7
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This depends on #141 so please review that first (or close it and merge this one)