Skip to content

Add a FileSystemLoader to load external Jinja templates#85665

Closed
DanielBaulig wants to merge 2 commits into
home-assistant:devfrom
DanielBaulig:enable-jinja2-import-include
Closed

Add a FileSystemLoader to load external Jinja templates#85665
DanielBaulig wants to merge 2 commits into
home-assistant:devfrom
DanielBaulig:enable-jinja2-import-include

Conversation

@DanielBaulig
Copy link
Copy Markdown

@DanielBaulig DanielBaulig commented Jan 11, 2023

Proposed change

Adds a Jinja2 FileSystemLoader to enable loading of external templates within Home Assistant Jinja templates. Effectively activates support for {% include 'template.j2' %} and {% import template.j2 as t %}, which in combination with macros allow for the easy user side definition and reuse of templates and template macros throughout Home Assistant, e.g.

config/templates/mymacros.j2

{% macro is_jolly(v) %}
  {{ "it is" if v == "jolly" else "it is not" }}
{% endmacro %}

Template Developer Tools

{% set value = "maybe jolly?" %}
{% import "mymacros.j2 as m %}
{{ m.is_jolly(value) }}

All external template files must be located in the "templates" subdirectory of the Home Assistant config directory.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
    I have two pre-commit hooks fail that I skipped using --no-verify.
    The mypy error I'm not sure I understand or how to address. The other one seems to be an infrastructure and not a my code problem.
    I'd love some help understanding and fixing these.
pylint...................................................................Failed
- hook id: pylint
- exit code: 127

/Users/dev0/projects/ha-core/script/run-in-env.sh: line 22: exec: pylint: not found
mypy.....................................................................Failed
- hook id: mypy
- exit code: 2

usage: mypy [-h] [-v] [-V] [more options; see below]
            [-m MODULE] [-p PACKAGE] [-c PROGRAM_TEXT] [files ...]
mypy: error: Invalid error code(s): annotation-unchecked
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
    I believe one of the pre-commit hooks ran black
  • Tests have been added to verify that the new code works.
    I have not added any tests so far.

If user exposed functionality or configuration variables are added/changed:

To help with the load of incoming pull requests:

  • I have reviewed two other open pull requests in this repository.
    I don't have a lot of python experience and am not sure if I'll be of much value here.

@DanielBaulig DanielBaulig requested a review from a team as a code owner January 11, 2023 05:54
@home-assistant home-assistant Bot added cla-signed core new-feature small-pr PRs with less than 30 lines. WTH Issues & PRs generated from the "Month of What the Heck?" labels Jan 11, 2023
@soloam
Copy link
Copy Markdown

soloam commented Jan 11, 2023

This would be one of the best additions to Home Assistant ever. Please don't let this go idle.

@balloob
Copy link
Copy Markdown
Member

balloob commented Jan 30, 2023

Duplicate of #42244 and #13535. See first one for discussion.

@balloob balloob closed this Jan 30, 2023
@DanielBaulig
Copy link
Copy Markdown
Author

@balloob Thanks Paulus for pointing me to the previous discussion of equivalent changes. If I would like to make the case in favor of the change and provide arguments for why I think this is something that should be considered to be merged into HA core, where would the right place be? It seems the two other mentioned pull requests have been locked.

In case here is the right place I would like to make an attempt at summarizing the main objections / open questions to these pull requests, just to make sure we're on the same page on what the perceived issue with this approach is. Please let me know if I am missing any major concerns / problems.

I've gathered all these points/questions from the two pull requests linked above.

  • Is it worth making the structural changes to HA?
  • Why not use a script?
  • Why not use the Python integration?
  • I/O is not allowed from within the event loop

I also see some alternatives suggested, in particular:

  • Adding support for custom filters (as a custom component or in HA core)

@balloob
Copy link
Copy Markdown
Member

balloob commented Jan 30, 2023

The major issue here is I/O in event loop. It halts the whole system so is not allowed. Needs to be done in executor. That means you need async_render to be turned from a callback to an async function, which causes any render function to yield. This is not acceptable because it would spread out a lot of jobs across multiple task yields, causing 2 templates to be rendered next to one another to no longer be guaranteed to be based on the same state machine.

@github-actions github-actions Bot locked and limited conversation to collaborators Jan 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed core new-feature small-pr PRs with less than 30 lines. WTH Issues & PRs generated from the "Month of What the Heck?"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants