Turn HTML Minification Off / Enable Whitespace Removal #148
Conversation
…-questionnaire-runner into add-jinja-whitespace-flags
app/jinja_filters.py
Outdated
| @@ -288,9 +289,11 @@ def __init__(self, option, index, answer): | |||
| # the 'pre-' prefix is added to the attributes here so that html minification | |||
| # doesn't mess with the attribute contents (the 'pre-' is removed during minification). | |||
| # see https://htmlmin.readthedocs.io/en/latest/quickstart.html | |||
| attribute_key = "pre-" if settings.EQ_ENABLE_HTML_MINIFY else "" | |||
There was a problem hiding this comment.
Should we not be getting the config from current app config like we do elsewhere instead of accessing in directly?
app/setup.py
Outdated
| application.jinja_env.trim_blocks = True | ||
| application.jinja_env.lstrip_blocks = True |
There was a problem hiding this comment.
We should consider grouping these with the other jinja environment changes from line 191, possibly in a setup_jinja_env method.
| - name: EQ_ENABLE_HTML_MINIFY | ||
| value: "False" |
There was a problem hiding this comment.
PR says default minification in runner to False, but this is only done on deployed envs atm. EQ_ENABLE_HTML_MINIFY in settings.py should be changed to "False" as the default. Which would mean this change could also be removed as it doesn't override the default.
There was a problem hiding this comment.
The reason it is disabled is to boost performance for census. If we were to disable it by default, we may as well remove minification entirely.
There was a problem hiding this comment.
The intent was to get it merged, see the performance impact on the daily test, then decide whether to remove it completely in a separate card.
What is the context of this PR?
The results of our performance investigation show that html minification has a significant effect on runner's response time. This PR defaults the minification in runner to false and enables whitespace removal through use of jinja flags.
How to review
Check tests continue to pass. Review performance investigation results here: https://github.com/ONSdigital/eq-survey-runner-benchmark/tree/use-non-minified-html