-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[#3833] Extend Flask template dirs #3860
[#3833] Extend Flask template dirs #3860
Conversation
Add additional template directories to template_loader during creation of Flask app stack. Previously we put list of additional dirs into `config['pylons.app_globals'].template_paths`, but as we are migrating to Flask I moved this value to `config['computed_template_paths']`. After this, inside `make_flask_app`, default `jinja_loader` replaced with ChoiceLoader, that will try to load templates from either default dirs or custom ones.
Generally, previous solution works, but template_loader created only once and not updated during dynamic plugin loading. New `CkanextTemplateLoader` inherits from `FileSystemLoader` but does not save path to template dir inside own property. Instead, property getter returns current state of config variable that contains template paths.
ckan/config/middleware/flask_app.py
Outdated
@@ -54,6 +57,13 @@ def make_flask_stack(conf, **app_conf): | |||
app.template_folder = os.path.join(root, 'templates') | |||
app.app_ctx_globals_class = CKAN_AppCtxGlobals | |||
app.url_rule_class = CKAN_Rule | |||
extra_paths = config.get('computed_template_paths') | |||
|
|||
if extra_paths: |
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.
Shouldn't we use the ChoiceLoader
always regardless of whether we have computed_template_paths
in the config at this point?
Otherwise if you start CKAN without any plugin loaded at all you won't detect templates loaded at runtime, like in tests. I know this is impossible right now as we have "core plugins", but wondering if it makes more sense to future proof this.
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.
Yeah, it has sense. I prefer to keep things as simple as possible, but here we can allow permanent ChoiceLoader even if there is only one tpl source
This looks fantastic @smotornyuk, just one tiny question |
Updated. |
Thanks @smotornyuk! Good to have green tests again |
Add additional template directories to template_loader during
creation of Flask app stack. Previously we put list of additional
dirs into
config['pylons.app_globals'].template_paths
, butas we are migrating to Flask I moved this value to
config['computed_template_paths']
. After this, insidemake_flask_app
, defaultjinja_loader
replaced with ChoiceLoader,that will try to load templates from either default dirs or custom
ones.