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

Added CEL to list of valid ExpressionLangs #188

Merged

Conversation

venera-program
Copy link
Contributor

What this PR does / why we need it:
According to the 0.8 spec, we shouldn't be verifying against a list of "accepted" ExpressionLangs, only that jq is the default one.

Serverless Workflow does not mandate the use of jq and it's possible to use an expression language of your choice with the restriction that a single one must be used for all expressions in a workflow definition. If a different expression language needs to be used, make sure to set the workflow expressionLang property to identify it to runtime implementations.

CEL is a commonly used expression language which sees use in implementations of this sdk-go library. Support for it as a valid ExpressionLang was removed in or around 4afc5f3.

Additional information (if needed):
Optimally, we would want the validation to be more dynamic than adding another language to the list (as I'm doing in this PR), but I do not know enough about the validation processes to present a robust enough solution for that.

@spolti
Copy link
Member

spolti commented Sep 12, 2023

That's fine by me.
@ricardozanini ?

@ricardozanini
Copy link
Member

Optimally, we would want the validation to be more dynamic than adding another language to the list (as I'm doing in this PR), but I do not know enough about the validation processes to present a robust enough solution for that.

@venera-program I agree. Ideally, we should remove the enum and accept any string, defaulting to jq.

Signed-off-by: Venera <31911811+venera-program@users.noreply.github.com>
@venera-program
Copy link
Contributor Author

@spolti
Could I get a merge if everything is approved?

@ricardozanini ricardozanini merged commit f50885c into serverlessworkflow:main Sep 19, 2023
6 checks passed
@spolti
Copy link
Member

spolti commented Sep 19, 2023

@venera-program do you need a new release?

@venera-program
Copy link
Contributor Author

@spolti
I'm vendoring from commit atm so it doesn't matter for me either way, up to you if a release is necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants