Skip to content
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

Fix {{..}} templating -- standardize on expr #9529

Open
alexec opened this issue Sep 6, 2022 · 2 comments
Open

Fix {{..}} templating -- standardize on expr #9529

alexec opened this issue Sep 6, 2022 · 2 comments
Labels
area/templating Templating with `{{...}}` type/feature Feature request type/tech-debt

Comments

@alexec
Copy link
Contributor

alexec commented Sep 6, 2022

The current templating system appears to be the primary cause of confusion and bugs. These are caused by the following:

  1. Template evaluation is unpredicable. By which I mean, you don't know when it'll evaluate and what the output will be. This can result in a workflow that passes just some of the time
  2. We use several evaluation technologies: Govaluate, fasttemplate, Expr+Sprig.

Minor issues:

  1. {{..}} does not play nicely with Helm templating.
  2. podSpecPatch is arguably a work-around rather than a solution.

It would be great to remove this entires class of issue.

To fix this we should:

  • Standardize on expr as the templating technology (easy).
  • Make evaluation time predicable - at field access time (hard).
@alexec alexec added the type/feature Feature request label Sep 6, 2022
alexec added a commit to alexec/argo-workflows that referenced this issue Sep 6, 2022
@agilgur5 agilgur5 added the area/templating Templating with `{{...}}` label Aug 18, 2023
@agilgur5
Copy link

agilgur5 commented Mar 6, 2024

I've written about my plans to standardize on expr in a few other places (#7576 (comment), #12694, #12693 (comment), etc), figured this issue should really be the central source of truth for that and so will document here.

  1. Close all related/duplicate issues (did this some months ago)
  2. "Soft deprecate" govaluate by removing it from the docs and examples and replacing it with expr everywhere, per Confusingly, when _kind of_ supports expr expressions #7576 (comment)
  3. "Hard" deprecate govaluate in 3.7 by detecting its usage (or non-expr usage) and logging out a deprecation warning saying to switch to expr
    • 3.7 is an example matching the current status: 3.6 is the next minor, so if the docs are updated in 3.5's time, give it one more minor to settle in
  4. Fully remove govaluate in 3.8 and error out when its usage is detected, saying to switch to expr
    • 3.8 is an example, should be at least one minor after the previous step

Per #7576, as when already supports expr (albeit with some confusing error messages), we can do this without a breaking spec change to the CRs

This covers govaluate but not quite fasttemplate (#7810) or text/template -- the same process could be followed for those. govaluate is arguably the most confusing one (based on all the issues around it etc) to focus initial efforts on. The other, non-expression templating systems are also documented in the same place as expr, so govaluate is kind of the outlier in that sense as it is only used in when.

@agilgur5 agilgur5 changed the title Fix {{..}} templating Fix {{..}} templating -- standardize on expr Sep 5, 2024
@agilgur5
Copy link

agilgur5 commented Sep 5, 2024

  1. {{..}} does not play nicely with Helm templating.

Syntax replacement has been spilt to #13471.

To be specific, this issue focuses on the expr standardization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/templating Templating with `{{...}}` type/feature Feature request type/tech-debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants