Skip to content

feat: Jinja2 template engine in workflow templates #4595

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

Open
wants to merge 80 commits into
base: main
Choose a base branch
from

Conversation

korolenkowork
Copy link
Contributor

@korolenkowork korolenkowork commented Apr 19, 2025

Closes #4594

πŸ“‘ Description

This PR added the whole power of Jinja2 templating to the KeepHQ workflow

βœ… Checks

  • My pull request adheres to the code style of this project
  • I have updated the documentation as required
  • All the tests have passed

✨ Features

  • Users can now choose their preferred template engine for workflows: Jinja2 or Mustache
  • Mustache remains the default template engine
  • Introduced a syntax validator that raises a RenderError if invalid syntax is used for the selected engine
  • If an incorrect templating engine is specified in the YAML, a ValueError is raised

πŸ›  Improvements

  • Mustache (via Chevrone) now uses a tracking dictionary to log missing keys in the context, implemented in a thread-safe way.

⚠️ Known Limitations

  • During Jinja2 rendering, only missing top-level keys can be detected β€” full key paths cannot be tracked.
    Example: {{ alert.namespase }} will report {{ namespace }} as missing, but due to how Jinja2's Undefined class works, it cannot identify the full path to the missing value.

  • It’s not possible to use a tracking dictionary for Jinja2 in the same way as Mustache, because Jinja2 internally converts the context to a plain dict during rendering.

Copy link

vercel bot commented Apr 19, 2025

@EnotShow is attempting to deploy a commit to the KeepHQ Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. Documentation Improvements or additions to documentation Feature A new feature labels Apr 19, 2025
@korolenkowork korolenkowork changed the title Feature/jinja template engine in workflow templates feature: Jinja2 template engine in workflow templates Apr 19, 2025
@korolenkowork korolenkowork changed the title feature: Jinja2 template engine in workflow templates feat: Jinja2 template engine in workflow templates Apr 19, 2025
@korolenkowork korolenkowork marked this pull request as draft April 19, 2025 18:22
@korolenkowork korolenkowork marked this pull request as ready for review April 19, 2025 22:53
@shahargl
Copy link
Member

@EnotShow lot of unit tests are failing, can you fix that? :)

Copy link
Member

@shahargl shahargl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests failing

@talboren
Copy link
Member

Also a quick question about backwards compatibility - not sure if I understand if this will still support mustache or we're replacing to Jinja completely? Maybe we should support templating=mustahce or templating=jinja in the top of the workflow to decide which engine we want to use. Will the keep. functions work?

@korolenkowork korolenkowork marked this pull request as draft April 20, 2025 08:46
@korolenkowork
Copy link
Contributor Author

@talboren

  1. This PR fully replaces Chevron with Jinja2 in workflows. I’ve proposed sticking with it, since once this is done, the previous workflows need to remain fully compatible. But it makes sense to proceed cautiously, especially since we don’t yet know all the potential issues this might introduce. So if the team decides it has to be done, I’m happy to go ahead with it.

  2. Yes, keep functions working perfectly!

Copy link
Member

@shahargl shahargl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems almost done, see comments.

and if you can add more tests it will be great <3 just a big change so want to make sure we are ok.

@shahargl
Copy link
Member

shahargl commented May 5, 2025

@EnotShow let me know when you want the final review + merging

@korolenkowork
Copy link
Contributor Author

@talboren This is ready for review, but I ran into an issue with Jinja2.

Jinja2 doesn't allow hyphens in identifier names when accessed as attributes in templates. To work around this, I implemented a function which replaces code like:

Pod status report:
{% for pod in steps.get-pods.results %}
Pod name: {{ pod.metadata.name }} || Namespace: {{ pod.metadata.namespace }} || Status: {{ pod.status.phase }}
{% endfor %}

with:

Pod status report:
{% for pod in steps['get-pods'].results %}
Pod name: {{ pod.metadata.name }} || Namespace: {{ pod.metadata.namespace }} || Status: {{ pod.status.phase }}
{% endfor %}

This change uses dictionary-style access, which is valid in Jinja2 for keys that contain hyphens. It seems to work, but I not sure if this solution is proper enough. What you think?

@talboren
Copy link
Member

@talboren This is ready for review, but I ran into an issue with Jinja2.

Jinja2 doesn't allow hyphens in identifier names when accessed as attributes in templates. To work around this, I implemented a function which replaces code like:

Pod status report:
{% for pod in steps.get-pods.results %}
Pod name: {{ pod.metadata.name }} || Namespace: {{ pod.metadata.namespace }} || Status: {{ pod.status.phase }}
{% endfor %}

with:

Pod status report:
{% for pod in steps['get-pods'].results %}
Pod name: {{ pod.metadata.name }} || Namespace: {{ pod.metadata.namespace }} || Status: {{ pod.status.phase }}
{% endfor %}

This change uses dictionary-style access, which is valid in Jinja2 for keys that contain hyphens. It seems to work, but I not sure if this solution is proper enough. What you think?

makes absolute sense! I'll try to review it ASAP.

Copy link

cursor bot commented Jun 14, 2025

🚨 BugBot couldn't run

Pull requests from forked repositories are not yet supported (requestId: serverGenReqId_06c76e83-cbf9-46fb-b4f7-80acc0b782b5).

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Test Fails Due to Incorrect Variable Usage

The test_workflow_execution_with_jinja2 function incorrectly references the undefined variable workflow_definition2 when creating the workflow object. It should use workflow_definition_jinja2, which is the intended Jinja2 workflow definition. This causes a NameError and prevents the test from properly verifying Jinja2 templating functionality.

tests/test_workflow_execution.py#L1692-L1693

interval=0,
workflow_raw=workflow_definition2 % workflow_id,

Fix in Cursor


Bug: Mustache Safe Rendering Regression

The MustacheIOHandler._validate_template method incorrectly disables safe rendering and logs a message about inverted sections for all Mustache templates. This is a regression caused by the removal of a conditional check that previously ensured safe mode was only disabled when actual inverted sections ({{^ or {{ ^) were present in the template.

keep/iohandler/iohandler.py#L695-L702

def _validate_template(self, template, safe):
self.logger.debug(
"Safe render is not supported when there are inverted sections."
)
safe = False
return super()._validate_template(template, safe)

Fix in Cursor


Bug: Template Engine Validation Fails When URL Shortening Disabled

The template_engine attribute validation is incorrectly placed within the self.shorten_urls conditional block. This prevents the attribute from being validated as defined when URL shortening is disabled, potentially leading to AttributeError later, as template_engine should always be defined regardless of URL shortening configuration.

keep/iohandler/iohandler.py#L85-L87

self.shorten_urls = True
if not self.template_engine:
raise AttributeError("template_engine is not defined")

Fix in Cursor


Was this report helpful? Give feedback by reacting with πŸ‘ or πŸ‘Ž

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation Feature A new feature size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[βž• Feature]: Jinja template engine in workflow templates
4 participants