Defer translated string evaluation on validators. by doctoryes · Pull Request #5452 · encode/django-rest-framework (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation6 Commits2 Checks0 Files changed
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
Description
This PR originated with the changes in this PR: #3438 . However, that PR stalled and was closed. This PR proposes the same change again - defer the error message string evaluation until the messages are actually used.
As stated in the original PR, our project uses a forked version of DRF only because of the issue described in #3354 . Our fork contains only one custom change - the one in this commit: edx@3c72cb5 Our project (Open edX) performs a non-standard startup which causes DRF to be imported before the Django translation infrastructure is initialized.
An initial attempt to fix this issue was made in this PR: #3407 - but was replaced by the more recent PR: #3438
@carltongibson @xordoquy FYI
@bmedx @macdiesel FYI
Closes #3354
Seems this went under my radar again.
Thanks for bringing it back !
PR needs to review but looks good at first sight.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Lets have this. What a nice one to clear up!
I added a docstring to CustomValidatorMessage just so we can remember what this is all for in two years time.
@doctoryes @xordoquy Thanks for the work on this! Good one. 👍
Customize validators to defer error string evaluation. Defer translated string evaluation on validators.
carltongibson pushed a commit that referenced this pull request
Set version number for 3.7.0 release
Rename release notes section
Moved issue links to top for easier access. (Can move back later)
Strict JSON handling
Add release note for #5250
Add release notes for #5170
Add release notes for #5443
Add release notes for #5448
Add release notes for #5452
Add release not for #5342
Add release notes for 5454
Remove Django 1.8 & 1.9 from README and setup.py
- Release notes for merged 3.6.5 milestone tickets
Tickets migrated to 3.7.0 milestone.
Add release notes for #5469
Add release notes from AM 2ndOct
Add final changes to the release notes.
Add date and milestone link
Move issue links back to bottom.
Update translations from transifex
Begin releae anouncement
Add release note for #5482
3.7 release announcement & related docs.
This commit makes serialization process 3 times slower. In my project, I inspected that serializing about 160 objects takes around 1500 milliseconds. After profiling, I've found out that the most time consuming tasks are related to using of Django's lazy function:
ncalls tottime percall cumtime percall filename:lineno(function)
9126 0.428 0.000 0.865 0.000 functional.py:82(prepare_class)
847785 0.343 0.000 0.343 0.000 {built-in method builtins.hasattr}
2724/2562 0.198 0.000 0.233 0.000 fields.py:627(deepcopy)
267546 0.065 0.000 0.065 0.000 {built-in method builtins.setattr}
4386 0.057 0.000 0.080 0.000 {built-in method builtins.build_class}
8244 0.056 0.000 0.071 0.000 fields.py:320(init)
232458 0.048 0.000 0.048 0.000 functional.py:102(promise)
4431 0.039 0.000 0.273 0.000 field_mapping.py:66(get_field_kwargs)
158386 0.039 0.000 0.045 0.000 {built-in method builtins.getattr}
795 0.031 0.000 1.729 0.002 serializers.py:989(get_fields)
4386 0.024 0.000 0.024 0.000 functional.py:57(proxy)
.
.
.
After removing the usage of lazy function in restframework source code, my serialization process finishes after around 500 miliseconds.
Hm. I think this can be resolved by relying on the ValidationError to format the message, instead of us. Basically, we need to change our error messages to use %(limit_value)s and let the ValidationError format the message, instead of providing a lazily formatted {max_value}. @mehdipourfar - do you think you'd be up for creating a PR?
Although, I guess this would create a compatibility issue w/ users who already override their custom messages with their own format strings. Maybe we could detect this? idk if there's a reliable way of doing so.
Then again, detecting this would defeat the purpose of lazy evaluation.