4 ms·
You seem to have stepped on the same landmine that Ansible did, by defaulting to the jinja2 [aka text/template silliness in golang] of using double mustaches in
by mdaniel 1y ago
You seem to have stepped on the same landmine that Ansible did, by defaulting to the jinja2 [aka text/template silliness in golang] of using double mustaches in YAML. I hope you enjoy quoting things because you're going to be quoting everything for all time because "{" is a meaningful character in YAML. Contrast
parameters:
status: "{{ var('order_status') }}"
with
parameters:
# made famous by GitHub Actions
status: ${{ var('order_status') }}
# or the ASP.Net flavor:
status2: <%= var('order_status2') %>
# or the PHP flavor:
status3: <?= var('order_status3') ?>
and, just like Ansible, it's going to get insaneo when your inner expression has a quote character, too, since you'll need to escape it from the YAML parser leading to leaning toothpick syndrome e.g.
parameters:
status: "{{ eval('echo \"hello\"') }}"
---
If you find my "but what about the DX?" compelling, also gravely consider why in the world `data_expression:` seems to get a pass, in that it is implicitly wrapped in the mustaches
---
edit: ah, that's why https://github.com/paloaltodatabases/sequor/blob/v1.2.0/src/sequor/common/executor_utils.py#L6 https://github.com/paloaltodatabases/sequor/blob/v1.2.0/src/... but https://github.com/paloaltodatabases/sequor/blob/v1.2.0/src/sequor/common/executor_utils.py#L138 https://github.com/paloaltodatabases/sequor/blob/v1.2.0/src/... is what I would suggest changing before you get a bunch of tech debt and have to introduce a breaking change. From
str_rendered = Template(template_str, undefined=StrictUndefined).render(jinja_context)
to
str_rendered = Template(template_str, undefined=StrictUndefined,
variable_start_string="${{",
variable_end_string="}}"
).render(jinja_context)
# et al, if you want to fix the {# and {%, too
per https://jinja.palletsprojects.com/en/stable/api/#jinja2.Template:~:text=variable_start_string https://jinja.palletsprojects.com/en/stable/api/#jinja2.Temp...
- maxgrinev 1y agoThank you for such an insightful suggestion and deep dive into the code - this is amazing feedback! I'll definitely switch to the ${{}} syntax you suggested. Quick clarification on _expression: we intentionally use two templating systems - Jinja {{ }} for simple variable injection, and Python *_expression for complex logic that Jinja can't handle. Actually, since we only use Jinja for variable substitution, should I just drop it entirely? We have another version implemented in Java/JavaScript that uses simple ${var-name} syntax, and we already have Python expressions for advanced scenarios. Might be cleaner to unify on ${var-name} + Python expressions. Given how deeply you've looked into our system, would you consider using Sequor? I can promise full support including fundamental changes like these - your technical insight would be invaluable for getting the design right early on.
- mdaniel 1y agoI'm not the target audience for this product, but I experience the pain from folks who embed jinja2/golang in yaml every single day, so I am trying to do whatever I can to nip those problems in the bud so maybe one day it'll stop becoming the default pattern As for "complex logic that jinja can't handle," I am not able to readily identify what that would mean given that jinja has executable blocks but I do agree with you that its mental model can make writing imperative code inside those blocks painful (e.g. {% set _ = my_dict.update({"something":"else}) %} type silliness) it ultimately depends on whether those _expression: stanzas are always going to produce a Python result or they could produce arbitrary output. If the former, then I agree with you jinja2 would be terrible for that since it's a templating language[1]. If the latter, then using jinja2 would be a harmonizing choice so the author didn't have to keep two different invocation styles in their head at once 1: one can see that in ansible via this convolution: body: >- {%- set foo = {} -%} {%- for i in ... -%} {%- endfor -%} {# now emit the dict as json #} {{ foo | to_json }}