-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Defer translated string evaluation on validators. #5452
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
Defer translated string evaluation on validators. #5452
Conversation
0797b26
to
24391db
Compare
Seems this went under my radar again. |
There was a problem hiding this comment.
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. 👍
* Set version number for 3.7.0 release * Rename release notes section Moved issue links to top for easier access. (Can move back later) * Add release note for #5273 * Add release note for #5440 * Add release note for #5265 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 * Add release notes for #5058 & #5457 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:
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 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. |
Hi @rpkilby |
Thanks. |
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