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(template): allow partially resolved vars in arithmetic expressions #6228

Merged

Conversation

vvagaytsev
Copy link
Collaborator

@vvagaytsev vvagaytsev commented Jun 25, 2024

What this PR does / why we need it:

This PR allows using partially resolved variables in arithmetic operations.

Which issue(s) this PR fixes:

Fixes #6201

Special notes for your reviewer:

If we omit the explicit double-quotes in the echo command args in the example in #6201, i.e. replace

spec:
  command: [echo, '"${var.prefix.suffix * 4}"']

with

spec:
  command: [echo, '${var.prefix.suffix * 4}']

we'll get a schema validation error:

Error validating spec for Run type=exec name=my-test:

At path spec.command.1: Expected string, received number

That is not related to the template string resolution and should be fixed in a separate PR. A possible solution is to stringify all numbers in spec.command's args implicitly.

@vvagaytsev vvagaytsev requested review from 10ko and twelvemo June 25, 2024 13:32
@vvagaytsev
Copy link
Collaborator Author

@twelvemo @10ko it's still a draft, sorry for the early review request.

@vvagaytsev vvagaytsev force-pushed the fix/template-var-resolution-in-arithmetic-expressions branch from 57a97a1 to 99c8d42 Compare June 25, 2024 13:39
@vvagaytsev vvagaytsev marked this pull request as ready for review June 25, 2024 13:51
@vvagaytsev vvagaytsev enabled auto-merge June 25, 2024 13:52
@vvagaytsev
Copy link
Collaborator Author

@twelvemo @10ko it's ready for review now :)

Copy link
Member

@10ko 10ko left a comment

Choose a reason for hiding this comment

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

Looks good!

@vvagaytsev vvagaytsev added this pull request to the merge queue Jun 25, 2024
Merged via the queue into main with commit 8d85a1a Jun 25, 2024
40 checks passed
@vvagaytsev vvagaytsev deleted the fix/template-var-resolution-in-arithmetic-expressions branch June 25, 2024 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.13: [Bug]: Template: Arithmetic binary operators do not work together with variable lookup
2 participants