-
Notifications
You must be signed in to change notification settings - Fork 1k
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
base: main
Are you sure you want to change the base?
feat: Jinja2 template engine in workflow templates #4595
Conversation
@EnotShow is attempting to deploy a commit to the KeepHQ Team on Vercel. A member of the Team first needs to authorize it. |
@EnotShow lot of unit tests are failing, can you fix that? :) |
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.
tests failing
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 |
|
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.
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.
@EnotShow let me know when you want the final review + merging |
β¦mplates Signed-off-by: Ihor <88674695+EnotShow@users.noreply.github.com>
@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:
with:
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. |
β¦mplates Signed-off-by: Ihor <88674695+EnotShow@users.noreply.github.com>
β¦mplates Signed-off-by: Ihor <88674695+EnotShow@users.noreply.github.com>
β¦mplates Signed-off-by: Ihor Korolenko <88674695+korolenkowork@users.noreply.github.com>
π¨ BugBot couldn't runPull requests from forked repositories are not yet supported (requestId: serverGenReqId_06c76e83-cbf9-46fb-b4f7-80acc0b782b5). |
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.
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
keep/tests/test_workflow_execution.py
Lines 1692 to 1693 in 225d43c
interval=0, | |
workflow_raw=workflow_definition2 % workflow_id, |
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
keep/keep/iohandler/iohandler.py
Lines 695 to 702 in 225d43c
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) |
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
keep/keep/iohandler/iohandler.py
Lines 85 to 87 in 225d43c
self.shorten_urls = True | |
if not self.template_engine: | |
raise AttributeError("template_engine is not defined") |
Was this report helpful? Give feedback by reacting with π or π
Closes #4594
π Description
This PR added the whole power of Jinja2 templating to the KeepHQ workflow
β Checks
β¨ Features
π Improvements
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.