Expressions vs. string interpolation (<strike>paths</strike>) #285
Comments
|
i dont really get this at all, but others maybe do so please chime in. for me its much less confusing just to say "if a string is supposed to be evaluated as an expression by the runtime put it inside ${ ... }". |
|
See my comments here on 'interpolation': |
|
Reposting this here for clarity: Consider this example from bash: What do you expect the output of env to include? Now consider the following example with how the spec currently handles this: toStateData: ${ .foo }Given the following action results: baz: 1Given the existing state data of: foo: .barPlease take a moment to consider how a user might think that they should be getting the following output because the expression foo: .bar
bar:
baz: 1Overall, I think there's something wrong about the way the spec is using the '${}'. In any other language I've seen, something like that is used for variable interpolation embedded in a string. In fact I think I saw examples where that's how it's being used (for example : I would propose the following be considered:
@tsurdilo & @neuroglia Could you please help me understand your point of view on this? |
|
Here's an snippet from the brigadier example that I think illustrates why this is confusing the way - name: NextEventState
type: event
onEvents:
- eventRefs:
- nextEvent
eventDataFilter:
toStateData: "${ .nextEvent }"
actions:
- name: consoleLogAction
functionRef:
refName: consoleLogFunction
arguments:
log: "fired ${ .nextEvent.data.type } caused by ${ .nextEvent.data.cause.event }"Imagine you're a user reading this spec for the first time and you see this example. Clearly the last line is trying to do some variable interpolation, but then...what's this |
|
Not getting a lot of response here, so I'll try again with an example workflow definition where the expression language feature is very ambiguous on how it should work. Given the following workflow definition ...
states:
- name: MyInjectState
type: inject
data:
a:
sub1: "foo"
b:
- "bar"
- "baz"
c: aString
transition: MyOperationState
- name: MyOperationState
type: operation
actions:
- functionRef
refName: myFunction
arguments:
f1: ${.a}
f2: ${.b}
f3: interpolating ${.a}, ${.b} and ${.c}
end: trueCan someone please explain what they think the payload to myFunction will contain? |
|
Can you please give some explanation for the example:
The result would always be the result of the expression applied to the associated input data |
|
Shoot, yah, this is a bad example. My intent is to show an example with the ambiguous examples side-by-side but this was the wrong way to express that. Reworking the example. |
|
@tsurdilo I updated the example to something that's hopefully more correct syntax. |
|
Here's what I would expect as the content of the arguments sent to the function: {
"f1": { "sub1": "foo" },
"f2": [ "bar", "baz" ],
"f3": "interpolating { \"sub1\": \"foo\" }, [ \"bar\", \"baz\" ], and \"aString\""
}Is this what others would expect? |
|
@jorgenj it's indeed the logical output that'd be produced with something equivalent in, say, js. In jq, though, it would result in an error, because as discussed today, jq only supports explicit strings in such cases. Even concatenating a number fails without explicit cast. There probably are methods to do so, I guess: I'm no jq expert. I agree however that this is confusing and might lead to unexpected/inconsistent behaviors across runtime. IMHO, interpolation should be delegated to expression language, or be defined explicitly (aka with some kind of markup), or be removed entirely as a feature supported by the spec. |
|
Thanks for your input Charles. I think I agree about the notion of potentially removing interpolation for now, I think there's some edge-cases that aren't clearly defined that will be confusing and like you hinted, it seems like a pretty complex feature for runtimes to implement. |
|
@jorgenj @neuroglia the results are what the expression returns, no magic. I would just use jq so there are no issues: "interpolating ${.b | tostring} and ${.a | tostring} and ${.c | tostring}" Idk why string concatenation would be too difficult to implement :( if result of expression is not a string and you want to concatenate it with a string, that would be same as in any programming language (would cause an error) |
|
Great, so given that example, here's the equivalent jq as you suggested:
In this case, some of the fields got the raw json value assigned, and in others interpolation was done. How is a user supposed to understand when interpolation will occur, vs. the raw value being used by the workflow? Put another way, for |
|
so in your example: f1 is f2 is: and f3: "interpolating {"sub1":"foo"}, ["bar","baz"] and aString" (with escaped quotes idk how to write it here) |
I think your expression returns a JSON value so its result would be JSON, so your parameter would have the JSON type when you mix text and expressions its string concatenation so results of expressions should be string type ( | tostring) hope this helps |
|
im pretty sure you can use regex to see if on runtime end you have text mixed in with expressions (${ .... }) and then |
|
Well, one concern here is that there seem to be pitfalls here for users of the workflow language:
It also seems like there's a situation where a user might want to force interpolation, as in they expect the expression |
|
we have to add expressions in string type properties i dont think there is another way. i would suggest for users that are non techical to use just expression functiosn and have devels set up expressiosn for them in function definitions so they can just use them by name and not have to write them themselves |
|
i honestly think that our DSL is by far superior to like other DSLs in this regard , and if you look at like bpmn its much much worse where you non technical users would have to write programming language code and all kinds of stuff can go wrong there. i think thats a problem in general with DSL that you kinda have to use an expression language. thats why we want to move to true DSL to get to a point where in control flow logic users can reference everything by logical names rather than having to type in expressions...we will get there eventually |
|
Thanks for the context, I agree that the DSL here is pretty neat, I'm just trying to clear up some confusion, please bear with me for a second. It seems like we generally have a common understanding of how interpolation should work so I'd like to then turn the question to the following example: ...
states:
- name: MyInjectState
type: inject
data:
a: .b
b: foo
transition: MyOperationState
- name: MyOperationState
type: operation
actions:
- functionRef
refName: myFunction
arguments:
...
actionDataFilter:
toStateData: ${.a}
end: trueLet's assume the function call there is returning the following payload: "Hello world"When that workflow completes, what would you expect the resulting stateData from MyOperationState to look like? Given that we've established above that when a user sees dollar-curly-braces ("${.a}") that this evaluates an expression, then it stands to reason that some users would naturally be confused if the output of the above workflow is not the following: {
"a": ".b",
"b": "Hello world"
}Edited to add: ie; the expression |
|
since .a already exists and has a value of type string, you can see "toStateData" as jq: .a += <results of function invocation>so in this case the state data should be: {
"a": ".bHello World",
"b": "foo"
}if the results of the function invocation is not a string, jq should raise you error (string and object cannot be added or something similar iirc) |
|
Hmm, it seems like you're ignoring the fact that |
|
sorry if i dont seem to see the issue . lets ping others to give opinions
as it seems im not much help here
…On Tue, May 11, 2021 at 1:51 AM Jorgen Johnson ***@***.***> wrote:
@tsurdilo <https://github.com/tsurdilo>
Hmm, it seems like you're ignoring the fact that "${.a}" looks like a
workflow expression. Are you suggesting that the toStateData field does not
act like all other workflow expressions in the DSL?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#285 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA5E7TB556INNT2WESX4JDTNDAUTANCNFSM4YVXZATQ>
.
|
is it not ? toStateData applies the action results and adds/merges it with with the state data value selected by the expression in this case it would be the top-level "a" element of state data.
i dont understand this question so idk if i am writing a runtime here or not but you could impl it with a jq expressions:
. += {"a": "value"}
.a += "value" This is a tiny bit more complicated when the selection expression in toStateData is not a top-level selection but perfectly doable imo |
If you were to put the same expression string in stateDataFilter.output it would produce the string ".b" in my example but in this context it somehow instead tells the runtime to put the results at ".a"? |
This is back to your original response, the point I'm trying to make is that some of these fields are clearly not being treated as a jq expression to be evaluated against some data, for example with actionDataFilter.toStateData it's not evaluating an expression to produce data but rather it's just giving a location where some other data should be merged.
In this case, by "all of the examples" I meant that things like Perhaps it'd help illustrate my point if I just push a PR with some of the examples being updated and we can review/discuss with some more concrete examples. |
It has to be an expression...thats the only way its possible to do anything productive with it. having just path is imo useful for custom, simple scenarios only. a simples example i can come up with is lets say we have a switch state {
"shoppers": [
{
"id": 111,
"age": 15
},
{
"id": 222,
"age": 21
}
]
}and function Y results (since we dont control them really...3rd party remember) gives us a slightly different result {
"buyers": [
{
"id": 111,
"age": 15
},
{
"id": 222,
"age": 21
}
]
}if later on in our workflow we want to add some event results or function results ..lets say add another person ..using just paths we have to hard-code "shoppers" or "buyers"...which can produce completely wrong results and even issues. with using expressions we can deal with third party services different results (that we dont control), for example: this basically says to select ".shoppers"..if shoppers is null (not and object) to use ".buyers" do that with path only :) there is a ton more examples like this that actually can solve real world scenarios. hope this helps. |
"numparam": "$expr .aNumber" fails this too (.aNumber can return an array/string/... ..but requires this super ugly way of writing it)
Why? its the same thing as having a method that returns Object and cast it depending on what you want (and it can throw exception if its not): |
|
i think possibly it would be good to be able to provide schema definition for state data..that way we could enforce property types. not easy to do but can take a look |
|
Unfortunately there's no type casting in JSON. It sounds okay, no? Anything that looks like
The first one is |
No, the
You're still missing the point. Your proposal does not provide any inherent way to produce a string that is the result of evaluating an expression with no prefix or suffix, and would therefore be a problem even if everyone liked the syntax.
It's not the same because you aren't providing any way to do the string cast! |
I don't see how it's just a way of expressing it whats the point? Only one is that our syntax uses the ${ expression } expression holder syntax that is widely adopted and "$exp expression" is from i can tell something you came up with. Json Schema uses this yes but i think its meant for parameter names not as a regex inside values? If we wanted to adopt something from Json Schema then I'd rather use the |
what string cast ..the example i show above does a string cast and even has the capability to check errors better has a return type of string because it contains characters/spaces in addition to the expression. this imo makes things even simpler for runtimes as as soon as you eval an expression that is not string return type you can raise an error |
|
The same syntax cannot be used for both string interpolation and type replacement—something like |
|
@gibson042 @manuelstein @cdavernas So what I would propose is these simple rules:
is a string ..nothing else.
even (as a replacement to example below) A concrete example can look like this: a) Top-level Function definitions: b) Then inside your states control flow logic: Since this string has that "special" expression format it itself cannot define an expression but can only be a reference to a single expression function. This way runtimes can map Runtimes can then also provide their own libraries where they can provide users with a list of pre-defined expressiosn that can be used if they want. I think this is the best idea as if if its not this thing that we are talking about regarding expressions it'll be something else in the future. Also this way we do no longer require even the use of "expressions" and i would change the function type "expression" to something else even like "customeval" or something. That said tho, for users that DO want to use expression inside expression type functions, they can still do it. WDYT? |
|
Are you suggesting that stateDataFilter/actionDataFilter/eventDataFilter no longer support expressions as well? Just trying to understand if this applies to any field anywhere in the spec where currently expressions are allowed, or only certain places? A clarifying question, is your item 1 there explicitly saying that support for interpolation is being removed from the DSL all together? |
No, no parameters in spec would change. filters work as currently. Runtimes just have the choice again to use expressions or use programming language. So benefit there too.
Yes it is removed with this approach from the DSL. Runtimes can interpolate all they want tho..up to you. |
Hmm, I would want a workflow to be portable and not have some unknown magic kick in on another runtime, so I don't think leaving it to the runtime is good for portability. Regarding the question:
I had a similar question in mind. Can I transform the data like this: or does it always have to be a function? Why should the content prefixed by Regarding
or even allow to create the entire list of states from a complex expression? Let's start with removing the confusion between interpolation and expressions.
In my opinion, only actions should be able to reference functions. Expressions are a different thing. |
|
@manuel
i think that after so many doscussions things get confused which i totally
understand.
the statement about runtimes interpolating was just a "joke" or "
statement" that runtimes in their code can do whatever they want, unrelated
to this and ok confusing but it didnt have anything to do with the subject
so ignore that.
expression type functions can be used in actions yes but that's even better as then you have even more reusability. that was the main reason we added them tobegin with.
i believe that what i proposed solves everyones issues:
1. removes interpolation in dsl
2 removes the need for multiple expressions inside prop values
3. even allows for easy way for runtimes to use programming lang rather
than expression libs
4. it does enforce more domain specific usage so plus there too as it
removes expressions from core dsl part which also enhance portability
5. provides a COMMON and SINGLE way to deal with expressions - this is really important as the proposals i heard about having multiple ways to define this / that...use "( )" or "{ }" or "$abc" in some case..this is all confusing people..don't u get it ? :)
i am not a fan of the "proposed changes" in your text. it just complicates
things and honestly i would rather keep it the way things are in that case.
Will create some examples for all to look at soon when i can.
…On Wed, Jun 9, 2021 at 7:29 AM Manuel Stein ***@***.***> wrote:
Runtimes can interpolate all they want tho..up to you.
Hmm, I would want a workflow to be portable and not have some unknown
magic kick in on another runtime, so I don't think leaving it to the
runtime is good for portability.
Regarding the question:
Are you suggesting that stateDataFilter/actionDataFilter/eventDataFilter
no longer support expressions as well?
I had a similar question in mind. Can I transform the data like this:
"stateDataFilter": { "input": "${ .customers = (.customers|map(select(.active))) }" }
or does it always have to be a function? Why should the content prefixed
by $expr or wrapped between ${ } not be an expression? Do you want to
allow multiple expression languages maybe, because I think that was not
part of this issue, but it would be convenient.
Regarding
If a property value has a special syntax, meaning it starts with "${ [...]
About that thing that we allow expressions in any property values (like the
example that determines the version at runtime
<https://github.com/serverlessworkflow/specification/blob/d36bcc5bd84e05c6ad904dd8918f6479653e1fdf/specification.md#workflow-expressions>).
I don't know if I we can stick with that degree of freedom. Do we allow a
state like this:
{
"name": "${ \"MyWorkflowNameWith-\\(.nameSuffix)\" }",
"type": "operation",
"actionMode": "${ .actionMode }",
"actions": "${ [range(4)] | map({ \"functionRef\": \"func\\(.)\" }) }",
"end": true
}
or even allow to create the entire list of states from a complex
expression?
We need to put some limits to where expressions can appear, otherwise we
might get some workflows that mutate along their execution and it causes
more confusion about when and how often the expressions should be
evaluated, e.g. timeouts: if my action timeout depends on workflow data and
that workflow data changes between actions, e.g. is increased by action
results, do I have to evaluate the timeout expression only when entering
the state or before every action? It's all possible, in theory, but it's
asking a lot from implementations to support such kind of sorcery and just
putting it out there leaves a lot to interpretation.
Let's start with removing the confusion between interpolation and
expressions.
We have proposals:
- A) Remove interpolation and only allow expressions
- A.1) Expressions are string values that start with $( and end on )
- A.2) Expressions are string values prefixed by a token $expr
- A.3) Expressions can only be used in workflow functions but can
be referenced
- B) Separate interpolation from expressions
- B.1) Use $() for expressions and %() for interpolation only
- B.2) Prefix the expressions "$expr .[] and bracket the
interpolations "Hello %( .name ), how are you?"
- C) Have a field type expression (like url has a special format or
uuid) and use it on fields where expressions are used, namely
- stateDataFilter.(input|output)
- actionDataFilter.(fromStateData|results)
- eventDataFilter.data
- functionDef.operation (if type=expression)
Also, we should distinguish them from the path type that points to
a unique location in the state data (or that is used as the left hand side
of the expression path = result):
- actionDataFilter.toStateData
- eventDataFilter.toStateData
In my opinion, only actions should be able to reference functions.
Expressions are a different thing.
We could use expressions as action functions, but I don't think we should
use any action function for expressions.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#285 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA5E7TZXD43YZ5K7RHEAMLTR5GBVANCNFSM4YVXZATQ>
.
|
|
Ok, better to go one step at a time. |
Ah OK, good question, so what I am saying is let's say you have two expression function definitions: "functions": [{
"name": "Customer Id",
"operation": "getCustomerId",
"type": "expression"
},
{
"name": "Customer Name",
"operation": ".customer.name",
"type": "expression"
}]string type properties then can either 1) be a string 2) reference an expression function with "special syntax", meaning it can be either "${ Customer Id }" or "$expr: Customer Name" or whatever is chosen (I prefer to keep it as it is now as its widely used syntax). From runtime perspective they can treat the "operation" value of the expression function as they feel like. If they want to treat it as an expression given the provided "expressionLang" setting thats fine..if they want to treat it as a string that maps to some internal code in any programming language thats fair as well...either way its return type is the return type of the function and thus becomes the type that is plugged back into the property. |
@manuelstein expressions can only manipulate state data not the workflow definition. |
There's an example in the section workflow expressions that modifies the workflow version attribute:
And of course, we want to use them to define the |
|
@manuelstein good point but thats setting the values of the workflow definition, my understanding is you said you could arbitrarily add workflow states, the structure of the workflow def cannot be changed. this uses in-line expressions in string type prop values. so we would change that to: "version": "${ Get Input Version }" where you would have to define an expression function that deals with that. yes this is a "pain" but after all this conversation it is a much cleaner solution than the stuff that was proposed imo. it enhances portability and reusability (you can also reference this "Get Input Version" function in other places) so +100 ;) The only thing I would also add in the future is the ability for expression type functions to define which "data" they are to be evaluated against. For example it can either be "state data", or "Workflow data inputs" for properties that are not inside data filters, for those its pretty clear what data input they are to be evaluated against. |
|
And just to add kinda like last point really...I do not think that what we currently have is really a problem. There are other types of possible solutions too. Since its not possible via JsonSchema to "tag" a property as an "expression type" property we could use a similar approach to what AsyncApi for example does with extension properties (by defining the "x-" prefix on them, we could do like "expr-abc" to tag them as such for example, for properties that we know must be expressions. this could completely eliminate the "special syntax" So I understand in the end that there is changes requested and I will work on setting up examples to show before-after and let's take a look. |
|
Yes, exactly, having a property type (solution C) would eliminate the syntax.
Wow, that's really good input. So far, I think we have enough arguments that the simplification causes real-life problems, but you make a very good point here. @tsurdilo Could you point to one or two big players' cases to put it in the balance? As I mentioned last week, I'm afraid I only know of cases that
JQ has string interpolation with It only requires us to decide how to do expressions. It aligns with how JSONschema uses the same approach to define the reference object type |
I'll second this, and specifically request an example of such a big player in which a value is a string in configuration but not necessarily a string at runtime, and a description of how that works. With respect to this issue, I suspect they are "not addressing it at all" because they don't have it—it doesn't exist if strings remain strings or are only used in contexts where the required data type is known and therefore conversion is well-defined. |
I don't know what you guys want from me really :) just look at ASL expressions syntax, for example This is really not how you request changes to any project. You address the issue with examples of whats broken and a PR on how to fix it in your opinion and then people can look at address concrete examples. Where the heck is any sort of respect for this project? Go to Kubernetes or any other project and start these types of conversations without a PR and concrete tests and then see if anyone in their right mind will respond to your there. We went above and beyond trying to figure this out. Maybe you need to start thinking "Am I not explaining things in a way that its fully understandable to others" before anything else. On the other hand with what I proposed I will do a PR and update all examples so you can nicely sit back and take a look at it and then put in all your comments on how you just don't like things w/o having a concrete proposal that actually works outside of your "world". Thanks ;) |
I'm not even clear on what the current behavior is, https://github.com/serverlessworkflow/specification/blob/d36bcc5bd84e05c6ad904dd8918f6479653e1fdf/specification.md contains many examples that use
Respectfully, it's on you to back up your claim ("I do also see some big players in this workflow DSL market not addressing it at all"). But I checked anyway, and it turns out that ASL does address this, because the places in their structure which are dynamically interpreted are strings that are checked for prefixes |
|
To clarify the ASL example:
I understand it looks like interpolation, but it's not. The parameter Personally, I think this has been text book so far: an issue has been raised, some convincing was done and it was triaged, so I'm going to assume for now that this has been verified as a real issue that needs solving. Luckily, through the extended discussion, we've collected many options:
1. Markup/prefix
2. JSONSchema style
3. parameter typing (like ASL)
|
|
Thanks all for your updates and great conversations!!! Many thanks for the great and very insightful information that you have provided. It will definitely improve our project in the long term!! |
|
How do you expect the project to improve by closing issues that are still unresolved? |
Uh oh!
There was an error while loading. Please reload this page.
What would you like to be added:
There are really 3 concerns at hand in this issue:
${}.Fix all the places where an 'expression' is used to specify a location in a json structure where data should be inserted/merged.Why is this needed:
Regarding string interpolation vs. workflow expressions, if both use-cases are basically a jq expression in a string surrounded by
${}, then there are situations where it's unclear which is intended. Imho, for fields where the purpose of the field is to accept an expression, the DSL should not require the${}, and that${}should be reserved to indicate string interpolation is expected.Recent changes to actionDataFilter confusingly use 'expression' for 'toStateData' attribute, and there are presumably many other places in the spec that also need to be fixed.Expressions (denoted by a string with${...}) typically extract or transform some values. On the other hand, a property like actionDataFilter.toStateData is expecting the user to indicate a 'path' or location in the state data where the results should be inserted/merged. It doesn't make sense to call this an 'expression', or to have the user style it like an expression. The proposal is to fix the descriptions, and remove the surrounding${}from all of the examples.The text was updated successfully, but these errors were encountered: