Skip to content

templateing: jinja2: pass kwargs for environment #427

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

Closed
blueyed opened this issue Mar 7, 2019 · 3 comments · Fixed by #1401
Closed

templateing: jinja2: pass kwargs for environment #427

blueyed opened this issue Mar 7, 2019 · 3 comments · Fixed by #1401

Comments

@blueyed
Copy link
Contributor

blueyed commented Mar 7, 2019

I think it would be good to pass something like env_kwargs via https://github.com./blueyed/starlette/blob/24c135de71ac56a73f7f797258115941579155bf/starlette/templating.py#L51-L53.

While you can change the env afterwards, it would allow Jinja2 to validate e.g. enable_async, and call load_extensions etc.

@tomchristie
Copy link
Member

Yup I'm up for this. We'll want to autoescape and loader be overridable there too.

Related to this is configuring env.globals.

We could just use **options, and support both globals and any valid Environment keyword arguments?

@blueyed
Copy link
Contributor Author

blueyed commented May 1, 2019

I think it should just allow more easily to set the env altogether.

url_for could be still set to the globals then by default.

But it is also the only reason for requiring request to be in the context (ValueError('context must include a "request" key')), and if url_for is not used in a template, it is not used (by default). I.e. it might be better to raise the error then from url_for only.

Not sure about backward compatibility with regard to removing directory from the constructor, but #473 might indicate that it is not used much (at least with tested code).. ;)

I am using a quite customized version already myself, and that is fine, but wanted to provide this feedback anyway.

@liquiddandruff
Copy link

+1 as well, since as currently stands it's not possible to change common fields such as auto_reload, enable_async when loading the environment, especially when deploying through a docker environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants