Dependencies and Enhanced Depends is broken if you call a task 'split' #12037
Comments
|
It also breaks if you call a task |
tico24
changed the title
caelan-io
added
the
type/regression
agilgur5
added
area/templating
|
|
We even did an argocon talk in April using it, so we know it worked then. |
|
If that's the case that 3.4.x worked with Argo variables overriding built-in functions, then it's possibly a regression in |
|
Does |
|
@tico24 Would you like to document this as a breaking change in upgrading guide? |
|
@terrytangyuan this seems more like a regression in |
|
I was gonna file a bug report in |
|
Agreed. This definitely isn't a breaking change, it's an issue. We also don't know the full list of magic bad words we can no longer use. If I found two in one workflow, I imagine there's lots. |
I imagine it would be every built-in function. And yes, that list could potentially grow with every |
|
IMO users should be aware of this when considering upgrading before major impact has occurred. |
|
Actually upgrading.md also contains non-breaking changes. |
|
|
agilgur5
added
the
solution/workaround
|
|
|
|
agilgur5
added
the
solution/suggested
|
After looking at the expr-lang/expr code, I am surprised how this ever worked actually. expr-lang has two phases, Our error happens in the compile phase (and rightly so) because split is of the wrong type. The fix for this should be from our end ultimately (imo), it's quite simple to do, replace all "${keyword}" expressions with "${argo_wf_keyword}" and then perform this replace in the env as well. We could also ask expr-lang to special case the Eval function, where we leak the env to the |
|
Created an issue: expr-lang/expr#520, let's see how they respond. |
I see. Let me refactor expr so env variables take precedence over built-in function (plus some syntax to still be able to call top overridden built-ins). |
|
Will create a new release with a fix. |
|
Now we need some syntax to access builtin function in case it is overridden by env variable. I'm thinking about split.Succeeded && ::split(longString, ",") |
|
Thanks @antonmedv, appreciate your quick response |
Yes, you can think of it as regexp: it also has compile and run steps. A good practice is to compile expressions on configuration update. And notify users of any errors. Also precompiled programs are faster. |
|
This has now been addressed in expr-lang, waiting for a new release, if urgent we can probably just use master. |
|
Will release today or tomorrow. |
|
I'm hitting the same issue, when running Metaflow on Argo with a @antonmedv was there a release? I'm hitting this with v3.5.4 thank you! |
|
Released new expr version https://github.com/expr-lang/expr/releases/tag/v1.16.0 |
There were indeed some type checker improvements in
This is potentially possible with |
|
When could we expect a release of Argo with the https://github.com/expr-lang/expr/releases/tag/v1.16.0 fix? |
|
There's a PR out above: #12573. That will almost certainly make it into the next patch release, 3.5.5 |
Signed-off-by: isubasinghe <isitha@pipekit.io> Signed-off-by: Isitha Subasinghe <isitha@pipekit.io>
agilgur5
added
the
area/upstream
Signed-off-by: isubasinghe <isitha@pipekit.io> Signed-off-by: Isitha Subasinghe <isitha@pipekit.io>
Signed-off-by: isubasinghe <isitha@pipekit.io> Signed-off-by: Isitha Subasinghe <isitha@pipekit.io>
Signed-off-by: isubasinghe <isitha@pipekit.io> Signed-off-by: Isitha Subasinghe <isitha@pipekit.io>
Signed-off-by: isubasinghe <isitha@pipekit.io> Signed-off-by: Isitha Subasinghe <isitha@pipekit.io> Signed-off-by: Isitha Subasinghe <isubasinghe@student.unimelb.edu.au>
Signed-off-by: isubasinghe <isitha@pipekit.io> Signed-off-by: Isitha Subasinghe <isitha@pipekit.io>
Signed-off-by: isubasinghe <isitha@pipekit.io> Signed-off-by: Isitha Subasinghe <isitha@pipekit.io>
Pre-requisites
:latestWhat happened/what you expected to happen?
If you happen to call a task
split, you'll break anything that depends on it.This is broken:
This is the same workflow, but with
splitrenamed tobar. It works fine:The resulting error in the UI is
This used to work on 3.4.11 and earlier. I'm even convinced it worked on both 3.5.0 release candidates.
Version
v3.5.0
Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.
See aboveLogs from the workflow controller
Logs from in your workflow's wait container
The text was updated successfully, but these errors were encountered: