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

Support for >=, <=, >, < conditionals. #860

Closed
vicaire opened this issue May 19, 2018 · 9 comments
Closed

Support for >=, <=, >, < conditionals. #860

vicaire opened this issue May 19, 2018 · 9 comments
Assignees
Labels
area/templating Templating with `{{...}}` type/feature Feature request
Milestone

Comments

@vicaire
Copy link

vicaire commented May 19, 2018

Is this a BUG REPORT or FEATURE REQUEST?: FEATURE REQUEST

Argo conditionals seem to only support "==":

https://github.com/argoproj/argo/blob/master/examples/conditionals.yaml

Would it make sense to add support for additional operations such as >=, <=, >, <?

For instance, in a machine learning context:

  • One step computes a model
  • The model is stored in an object store
  • the step outputs the accuracy of the model.
  • We could then use the ">=" to express that the workflow should proceed to the next step only if the accuracy of the model is greater than X.
@vicaire
Copy link
Author

vicaire commented May 19, 2018

@qimingj

@jessesuen
Copy link
Member

Would it make sense to add support for additional operations such as >=, <=, >, <?

Yes. The conditional parsing in Argo is extremely dumb at the moment. To go even further, we should support grouping with parens. Kubernetes already provides a very nice parsing library for this:
"k8s.io/apimachinery/pkg/labels"

This library is already being used in Argo's resource templates to support success conditions and failure conditions, but has not yet made its way in the controller conditional evaluation logic.

@vicaire
Copy link
Author

vicaire commented May 24, 2018

Grouping with parens? Would that be to support multiple output parameters?

@jessesuen
Copy link
Member

Right. You should be able to do something like:
(foo == true) && (bar == true)

@jessesuen jessesuen added this to the v2.2 milestone Aug 7, 2018
@jessesuen jessesuen self-assigned this Aug 7, 2018
@jessesuen
Copy link
Member

Implemented in 9b5c856 using the library: https://github.com/Knetic/govaluate. Here are some examples of what is now supported:

		"foo == foo",
		"foo != bar",
		"1 == 1",
		"1 != 2",
		"1 < 2",
		"1 <= 1",
		"a < b",
		"(foo == bar) || (foo == foo)",
		"(1 > 0) && (1 < 2)",
		"Error in (Failed, Error)",
		"!(Succeeded in (Failed, Error))",
		"true == true",

@vicaire
Copy link
Author

vicaire commented Aug 7, 2018

Thanks jessesuen!

raaidbroad added a commit to DataBiosphere/clinvar-ingest that referenced this issue Sep 11, 2020
raaidbroad added a commit to DataBiosphere/clinvar-ingest that referenced this issue Sep 15, 2020
* in progress

* wow

* add pipelineVersion to export-diff.yaml

* fix helm variable typo

* correct the kafka .Values input location

* correct the accessKey .Values input location

* fix access key thing in more places

* fix access key thing in more places

* try to add quote to fix yaml to json issue

* try to add quotes to fix yaml to json issue

* try to reformat template

* refactor so there is no template reference for preceding date

* add dependencies

* take out problem line and see what happens

* remove env variables to see if it fixes it?

* remove bad step?

* put problem back in, maybe it was because gcsPrefix was empty

* whitespace between source lines

* idk

* 🤷

* giving up for tn

* comment out problem workflows and see if we still get this stupid error

* uncomment date-present.yaml and references, replace line before line 42 to see if there is something weird being inserted

* delete the date-present workflow

* add input params to scripts in export-diff.yaml

* break out is-preceding-date-present into a template and refer to it

* pass in release-date to the new template reference

* fix indentation

* remove commented code

* remove unnecessary input block

* fix more indentations

* fix template use...

* add back in first draft of date-present.yaml

* fix env indentation, bad from copy-paste

* put in actual image name for date-absent, and try out a workflow level argument pass-in for date-present

* fix some indentation

* fix env variable passing in for kafka step

* update preceding date script

* fix bash script

* change template to trigger change?

* fix when logic

* can't use workflow parameters the way I wanted to

* refactor date-present.yaml

* forgot to point to the template to use it 🤦

* bump docker image version

* try a different way to run the python commands

* mess with commands more

* fix volume mounts, move get-preceding-date step to not run a million times

* fix awful copy-pasting

* remove useless input to get preceding date

* remove useless input to get preceding date

* remove redundant dependency

* fix typo GAH

* fix dataset name for python env vars

* fix missing period

* update docker image version

* update docker image

* add release date write step and update docker

* update date-absent workflow

* fix typo

* add dest-path to absent export

* shift some indentation

* remove some helm stuff to see what changes

* fix template indentation

* try to add better logging and error handling to bq steps in docker image

* fix import in image

* add more logging

* fix env variable passed in

* break out kafka to its own template for testing purposes, make corresponding changes, add output parameter to process-and-reingest-release.yaml

* fix indentation

* update kafka script

* fix some conditionals

* add a default value for the datasetname output

* fix default param

* add another default val

* update default param to be a string not a bool

* bool statement instead of a bool?

* statement works, should be false instead of true

* try to fix logic issue in workflow execution

* don't quote number

* double templating is so fun.

* pull kafka notifier into export-diff.yaml, eliminate processingNeeded as an output and just generate it again in the export-diff workflow. Try to add conditional when statements using go templating "and", no idea if it will work

* remove quotes from ands?

* try different syntax based on argoproj/argo-workflows#860

* write out the argo params in a string instead of trying to use the helm variables

* add processing-history template to export-diff.yaml

* fix indentation on source for get-processing-history-rows

* try adding even more quotes?

* make sure is-preceding-date-present always runs for downstream logic

* move get preceding date into date-present

* add get-preceding-date dependency to get-created/deleted/updated-rows

* fix indentation error in end of date-present.yaml

* add dependency

* update boolean check; export-diff should run stuff if there IS processing history, not if there isn't!

* update it in more places
@sylock
Copy link
Contributor

sylock commented Mar 19, 2021

The implemented functionality is really nice but the documentation should be upgraded to explain it. Currently the documentation is very poor about it. You only have :

In conclusion: this current issue is the only place where you can find a good documentation about the conditionals syntax.

@simster7
Copy link
Member

@sylock Thanks! Would you like to submit a PR with better documentation about this?

@julien-blanchon
Copy link

I didn't manage to make this work using 👍

when: {{steps.predict.outputs.artifacts.score}}"

I get the following workflow error:

Invalid 'when' expression '{{steps.predict.outputs.artifacts.score}} > 0.5': Invalid token: '{{' (hint: try wrapping the affected expression in quotes ("))

@agilgur5 agilgur5 added type/feature Feature request area/templating Templating with `{{...}}` labels Sep 20, 2023
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
Projects
None yet
Development

No branches or pull requests

6 participants