Identifying strings are expressions in place where the string might also be a constant

[fjtirado]

Currently we can write

set: 
  name: Javi

or

set: 
   name: ${.input.name}

However $ is a reserved keyword in JQ and {} are used in json, think of this expression written in json

"set": {"name": "${$loopVariable}"} 

(I wrote it wrong three times :))

I think we should consider following options

1. Everything is an expression

This implies user has to escape literals.

set: 
  name: '"Javi"'

set: 
   name: .input.name

2. Change the expression prefix.

Use something different than ${} for example, use ``

set: 
  name: Javi

set: 
   name: `.input.name`

We can use a different character, for example, #, but with this one we need to add white spaces around the expression

set: 
   name: # .input.name #

3. Use a new keyword.

So after the property name, you expect either constant or expr and use as default expr (that default might be overriden by user in the header of the workflow definition)

• Default to expr:

set: 
  name:
    constant: Javi

set: 
   name: .input.name

• Default to constant:

set: 
  name:  Javi

set: 
   name: 
     expr:  .input.name

[gibson042]

Relevant prior discussion: #285 (#285 (comment) even includes several of these options)

[cdavernas]

@fjtirado as discussed in PM, I totally understand your point of view, which is documented insightfully by others in the issue shared by @gibson042 (thank you!).

As I have never been bothered by the current approach, which I actually like, I'm probably not the most objective person when it comes to that subject.

I did, however, my due diligence, and have checked what major clouds are proposing, and it seems they are all using a similar mechanism. Hell, Google Workflows, which is my personnal favorite, uses the exact same than us.

Google Workflows, however, recommends single quoting expressions in YAML (which is/should be the authoring language) to avoid many of the problems you guys mentionned.

My proposal is:

1. Document single quoting recommendations
2. Optionally provide a top level property that allows configuring the workflow's expressions. Something like:

document:
  dsl: '1.0.0-alpha1'
  namespace: example
  name: example-workflow
  version: '0.1.0'
evaluate:
  language: jq
  expressions: explicitly | implicitly

Where explicitly defines that expressions must be clearly identified by '${ }', save when they are not explicitly required (as opposed to a when/exceptWhen condition, for example).
implicitly defines that everything is to be considered and run as an expressions. Constants must be clearly identified.

Also, I'm not against replacing the actual '${ }' with something else, as suggested in 2.

Finally, while I believe 1 is a great way to go, too, I think it is best "enabled" using my proposal, rather than by default. The reason is that, in the end, this issue is really a matter of tastes, perceptions and habits. Enforcing one or the other (such as it has been done so far) is IMO not the way to go.

What I would avoid is:

3. Use a new keyword.

It's more complicated, less readable, less "user-friendly", and a IMO a PITA to implement (where before I could simply regex my way out of it, I must know define special constructs to "extend" normal properties).

My opinion is that option costs much more than it gives.

---

As for suggestions to replace the current approach with an expr prefix, I believe it bears far too much "technical" meaning, which might put off "business people", who have a lot to gain with workflows, too.

[fjtirado]

@cdavernas
Google Expressions does not use JQ, thats why starting with $ is not an issue.
The model is not a json structure, thats why using {} is not an issue either.
Maybe it was lost in my long description but this expression in json looks really confusing when defining the workflow in json.
"set": {"name": "${$loopVariable}"}
We at least need a different enclosing set of characters.
For example
"set": {"name": "#$loopVariable#"}

[cdavernas]

when defining the workflow in json.

Well, why would you go through the sheer pain of defining anything in JSON when you can do it in YAML, I wonder?

[fjtirado]

@cdavernas
Believe or not, for me is easier to use {} (json) than tabs (yaml) (I am suspect that for other developers that consider that python practise a bad joke)
Regardless my preferences, since the definition file can still be written in json and even in yaml you might want to write expressions that return a json, for example
set: '${{loopVariable:$loopVariable}}'
I am really surprised thay you consider ${} a good separator for a specification using JQ and Json data everywhere just because it was done that way before (specially when we are defining a completely new DSL)

[cdavernas]

Believe or not, for me is easier to use {} (json) than tabs (yaml)

I believe you, and that's your right. But I am convinced that it will probably not be the case of the majority of our (especially non-technical) audience, which I'd rather focus on (as per design).

I am really surprised thay you consider ${} a good separator for a specification using JQ and Json data everywhere just because it was done that way before (specially when we are defining a completely new DSL)

Well, first, it's a matter of taste and colors, really. Second, I was looking to what was done where and why. The reasons identified and addressed by our "competitors" is of high interest to me, especially Google Workflows, which I admitted to be really fond of, on a purely syntaxic perspective.

The fact is that while having to build a JSON string should be doable, it's rather something you'd like to avoid, as you'd loose control over the data, which becomes opaque to any component of you workflow.

Please rest assured I read through and through the discussion we had on slack, the issue #285 and the new example supplied above. My opinion/taste remains the same nonetheless.

I therefore propose we all avoid trying to convince each other, and rather provide constructive solutions that could address the issue at hand!

I still believe we are speaking of edge cases twhich can be addressed in a way that becomes painfull for standard cases.

Let's remain concise in this issue, and move discussions to... discussions. I propose we only make and discuss proposals in here!

On a final note, it might be interesting to document those kind of fundamentals issues in ADRs, what do you @ricardozanini
@JBBianchi @fjtirado @gibson042 think?

[fjtirado]

Given your personal preferences, my proposal is to change ${} by ## $() (which is more neutral in jq and json)
Updated
See #848 (comment)

[cdavernas]

@ricardozanini @JBBianchi @gibson042 @jorgenj WDYT?

[fjtirado]

In #285 I think we were both discussing (makes sense at that moment) expression interpolation (which we are not supporting anymore, since we are deferring to the expression language) with expression identification (and the fact that were forcing the usage of ${} in places were it was not really needed because it was obvious the string was expression, problem that is not there anymore)
The only problem that remains is to identify expression in places where it is not evident that the string is an expression or a literal. So far, in the new DSL, I believe that will only occur in few places (set: and body:). We still have the same options that were collected in this comment, but we can ignore pros/cons related with interpolation and focus on identification.

[fjtirado]

and now my two cents, The markup proposal (either ${} or ##), since it can be used for interpolation, does not clearly send the message that we are NOT supporting it.
Since we just want to identify expressions, and, in my opinion, most of the time the user will be using expression on set: and body: rather than literals, I think I prefer option 3 (this is just personal preference, since I do not foresee implementation problems, but you never know till you actually implement it).
However, I agree that this creates another problem, which string to use as keyword so it does not collided with a "real" property? (but "$expr" and "$const" looks like a safe bet in that regard )
Since there is not unanimity in adopting 3., at the end, rather than pushing for it, I prefer to just change the markup we chose, clearly document that we are not supporting expression interpolation and allow user to indicate (as @cdavernas proposed) that all expressions are implicit
Therefore, we can adopt #848 (comment) , also explicitly documenting that interpolation is delegated to the expression language

[fjtirado]

I realize # is a poor choice for yaml because is the comment character (json does not have comment character and it was a good choice for it)
I would say we should consider using different expression markups for json and yaml if this were not complicating automatic conversion between the one and the other. So thats not a good option either
Given those constraints I would use $(), since neither $, ( and ) are special charaters in yaml/json (it still contains the $ that is used in jq, but the example I mention looks better in my opinion, because the braces to enclose the expression are different from the braces that enclose the json object)
"set": {"name": "$($loopVariable)"}
Therefore I propose to change the markup from ${} to $()
This allows you to write expression in yaml without actually quoting (or adding white spaces) them (as long as you are not building a json object or the expression does not contain a math operator)
Therefore, in yaml you will write

set: 
  name: $(input.name)

when currently you have to write

set: 
  name: ${ input.name }

or

set: 
  name: '${input.name}'

[ricardozanini]

1. +1 for the proposal to set implicit | explicit at the beginning of the workflow definition. We might explore the usage of expressions: strict or strict: true since it's widespread among many languages. Again, a matter of choice since we all agree that we need this feature.
2. +1 to change from ${} to $() to avoid misinterpretation with JSON expressions. Although one can argue that () is super common in programming languages that might also bring misinformation to the expression. From jq manual: Parenthesis work as a grouping operator just as in any typical programming language. Again, a matter of choice.
3. Welcome back @gibson042, please stay around

[fjtirado]

@ricardozanini what we are doing is writing an string that contains an expression that is evaluated.
() can be understood as synonomous to evaluate/call and $ might be understood as a shortcut for expression.
And JQ is a language, in practise, which is pretty () free (you write .number1 |sqrt rather than sqrt(.number1))

[ricardozanini]

@fjtirado I'm just adding to the discussion, I already agreed with ().

[fjtirado]

Also, we should be more strict defining what the expression should be returning.
In the cases we are discussing, set: and body: the expression is returning any valid json node (object, array or literal, this is why hardcoded number or string are also possible there, not just expressions)
In the case of switch, the expression should return a boolean literal.
In the case of username and password for basic authorization, the expression, if present, should return a string literal.
In the case of any timeout, the expression, if present, should return a number literal.
And so on.

[fjtirado]

Ok, it seems we have an agreement. Once @cdavernas is done with the DSL refactor, I will open a PR associated to this issue modifying the spec and the examples.

[ricardozanini]

@fjtirado I'll introduce examples + CI here: #852 #850.

[cdavernas]

Ok, it seems we have an agreement. Once @cdavernas is done with the DSL refactor, I will open a PR associated to this issue modifying the spec and the examples.

I'm not necessarily for that change, I don't think that using '$( )' is less confusing than '${ }' or causes less problems, especially with the loose mode added.

Plus, like discussed with @ricardozanini, I'd like before we make any decision to get @JBBianchi's input, which will be possible next week or so.

Ideally, we should gather as much "opinions" on this to see which one "wins", if any. Maybe setting a small poll?

[fjtirado]

@cdavernas It not a question of personal choice. Using {} as markup character in json is confusing and in yaml force you to use '' or additional spaces even when they are not needed. I think the rationale and the example I wrote here was pretty clear. I agree we need to vote, since keeping {} for me is a mistake, not a matter of personal taste

[cdavernas]

AFAIK, in YAML you do not have to do anything, because happilly it starts with ${, not simply {.

The reasoning behind using mustaches, which people can understandingly be nervous about, is because it's conventionnally used to indicate evaluation.

I'm not saying we should/should not use it, - hell, I like your suggestion-, I'm just saying I would like other people opinion before claiming unilaterally that we have an agreement 😛

Can we just wait for start of next week before taking a decision/acting?

[fjtirado]

In YAML you need to either use white spaces or quoutes, since { and } are special characters that needs to be esscaped. while ( and ) are is not, thats the whole point (for Yaml, I hope that for json is obvious that {} are a poor choice, the point can be summarized as " do not use as markup characters that are reserved in the language you are using to write the definition") and thats why in the examples I mentioned, you do not need quotes or white spaces anymore.

[fjtirado]

I agree we can wait till next week, I was raising the point becuase there is another PR where the ${} appeared (the looss evaluation one) and we need to wait to have an agreement before that PR is merged

[cdavernas]

In YAML you need to either use white spaces or quoutes, since { and } are special characters that needs to be esscaped

This is valid, with no quotes, no whitespaces:

test:
  expression: ${ Hello, world }

test:
  expression: ${Hello, world}

Am I missing something?

I dont know why you think you had to add the quotes in your examples, as this is valid, too:

set: 
  name: ${input.name}

Like I said, it's valid because the line starts with $, which is not a control character, and which makes following ones to be included as a string, whatever they are.

[cdavernas]

I agree we can wait till next week, I was raising the point becuase there is another PR where the ${} appeared (the looss evaluation one) and we need to wait to have an agreement before that PR is merged

That PR already was merged, as part of the refactor! I added the loose/strict before pushing it in, based on your/ric's awesome suggestions!

[fjtirado]

test:
  expression: ${ Hello, world }

should be valid because the special character { and } are separated by whitespaces

test:
  expression: ${Hello, world}

should not be. If it is, maybe the parser you are using is smart and interpreting that {} are not special character but part of the string because the string does not start with {, but my point still stands, { and } are reserved characters in yaml that should not be used for markup unless there were not any other option, which hopefully exist, because ( and ) are not reserved. And also avoid the uglyness when writing in json. So the advantages of replacing { by ( and } by ) should be clear.

This is the yaml spec section I was using https://yaml.org/spec/1.2.2/#742-flow-mappings

[cdavernas]

should not be (if it is, maybe the parser you are using is smart and intepreting that {} are not special characted but part of the string because the string does not start with {, but my point still stands

I dont want to be a PITA, especially that, once again, I'm not opposed to the change, but I disagree when you say your point still stands.

As a matter of fact, YAML doesnt tell you you shouldnt be using mustaches, it just tells you that if you encounter this token while reading, and before the token has been identified as, say, a string, it should denotate the start of a mapping.

If the line starts with, say, $, the token IS a string, and YAML allows you to write whatever you want, including mustaches, semi-colons, dots, which all are other control characters.

That fact goes against your point, saying that we can't use { because it's a control character. We are not using '{', we are using '${', which simply works!

On a side note to that, I want to remind you mustaches are what Amazon and Google have opted for. I'm not saying, once again, that we should use them, or that this fact is a good reason to use them, I just think that if YAML intended such limitations (which it does not, every linter I tried works without quotes), high chances are they would have chosen other characters.

[fjtirado]

if yaml is so smart and you can write everything after the $, then why we write the swith example using quotes
when: '.color == "red"'
If if starts with a ., it should be a string, isnt it?
But I would be happy if you point me to the specification section of yaml when thats stated so we can be sure that ${ working without qoutes is not a side effect of a particular yaml parser understanding but an universal fact.
Anyway, I think the yaml discussion is pointless, event if ${ is guaranteed to work, { and } are a visual problem when writing in json, as I tried to explain (apparently unsucessfully)

[fjtirado]

And now, lets discuss about google workflows
This is a snippet of one of their examples

body:
        kind: "sql#database"
        charset: "utf8"
        collation: "utf8_general_ci"
        name: ${database_name}
        instance: ${instance}
        project: ${project}
        sqlserverDatabaseDetails:
          compatibilityLevel: 5
          recoveryModel: "Simple"

They are using ${} for their expression without qoutes (so it seems ignoring { and } in indeed universal, as you claimed, my bad, I will review the yaml spec to check where that is stated) but they are significantly wrapping string constants within "" (I do not know why do they that)
In their expressions, they are not using json constructs (thats why { is not going to be ever an issue for them) and they are forcing the qoutes (so they are fine with escaping qoutes in json)

[fjtirado]

@cdavernas Btw, I do not think we are being a PITA. When writing a spec we need to be rigorous. I found this conversation enriching

[cdavernas]

But I would be happy if you point me to the specification section of yaml when thats stated so we can be sure that ${ working without qoutes is not a side effect of a particular yaml parser understanding but an universal fact.

I cannot do that, unhappilly, nor could you do the opposite. I'm however open to conceive an interpreter or another might not handle the use case you presented properly. Could you find one that didnt like the ${ ?

{ and } are a visual problem when writing in json

That is a very valid point, and is probably the essence of your proposal.
To play the devil's advocate, one could however argue that a, it is a matter of perception, b, that you should be writting in YAML, not JSON, but that also is a matter of personnal preference.

Btw, I do not think we are being a PITA. When writing a spec we need to be rigorous. I found this conversation enriching

Cool! And so do I!

[fjtirado]

I was playing a bit with the current conversion between YAML and JSON that I wrote using Jackson.

YAML snippet

       mantra: Serverless Workflow is awesome!
    boolean: true
    number: 2
    json: {"pepe":2}
    jqexpr: ${{pepe:2}} #jq expr that returns json object {"pepe":2}
    jqexprusingdifferentmarkup: .{{pepe:2}}

is converted to this JSON

       "mantra" : "Serverless Workflow is awesome!",
      "boolean" : true,
      "number" : 2,
      "json" : {
        "pepe" : 2
      },
      "jqexpr" : "${{pepe:2}}",
      "jqexprusingdifferentmarkup" : ".{{pepe:2}}"

This confirms that very likely any yaml parser which found any character before the { will understand that { is not declaring a json

[fjtirado]

Also
YAML
when: .color == "red"
JSON
"when" : ".color == \"red\""
@cdavernas So quoutes are not needed to wrap when condition

[cdavernas]

@fjtirado do we still need to do anything here, even after adding the evaluate top level property and related options?