[Feature Request] Document and validate that keyword args are disallowed in certain contexts

[cretz]

Describe the solution you'd like

Keyword arguments are not supported in activities, workflows, signals, queries, or updates. We should clearly document this in README and validate this rule at definition time with a clear error.

[santiagodoldan]

@cretz are there any plans on adding support to keyword arguments? We want to migrate from coinbase SDK and this would affect all our existing workflows and activities.

[cretz]

There are no plans to support keyword arguments because Temporal does not support them and many languages that Ruby SDK can invoke workflows/activities on do not support them. Therefore no Temporal SDKs in languages with keyword arguments (e.g. Python) allow them in workflows/activities. We usually encourage a single parameter that can have fields added which is compatible across all of Temporal.

I am just reading coinbase/temporal-ruby#255, but struggling to understand it a bit probably because I don't understand how that SDK serializes inputs. It seems to assume that all forms of input as an array whose last value is a hash means it's kwargs? I am curious how that would work if I invoked an activity with a single arg of ['foo', { 'bar' => 'baz' }] though I admit I didn't dig deep enough to see if that situation was properly disambiguated.

[santiagodoldan]

It's because the input goes inside another array 👇

[
  [
    "foo",
    {
      "bar": "baz"
    }
  ]
]

I am just reading coinbase/temporal-ruby#255, but struggling to understand it a bit probably because I don't understand how that SDK serializes inputs. It seems to assume that all forms of input as an array whose last value is a hash means it's kwargs? I am curious how that would work if I invoked an activity with a single arg of ['foo', { 'bar' => 'baz' }] though I admit I didn't dig deep enough to see if that situation was properly disambiguated.

[cretz]

It's because the input goes inside another array

Not following exactly. How can you differentiate ('foo', bar: 'baz') and ('foo', { 'bar' => 'baz' }) in this case? One is a positional and keyword parameter, another is two positional parameters, and it's important to know which is which at deserialization time.

All Temporal SDKs only support positional parameters, and all Temporal SDKs strongly encourage a single parameter that is an object/dict that can have fields added.

[santiagodoldan]

I did a little bit more of investigation

Coinbase SDK

Temporal.start_workflow(FooWorkflow, ['foo', { bar: 'baz' }])

Temporal.start_workflow(FooWorkflow, 'foo', { bar: 'baz' })

Temporal SDK

client.start_workflow(FooWorkflow, ['foo', { bar: 'baz' }], id: SecureRandom.uuid, task_queue: 'general')

client.start_workflow(FooWorkflow, 'foo', { bar: 'baz' }, id: SecureRandom.uuid, task_queue: 'general')

The only difference so far is the extra : which I don't know why they add it. There is almost the same behavior here which is great. The difference is how each SDK treats that input.

Not following exactly. How can you differentiate ('foo', bar: 'baz') and ('foo', { 'bar' => 'baz' }) in this case? One is a positional and keyword parameter, another is two positional parameters, and it's important to know which is which at deserialization time.

That's the magic of the double splat operator, it will support both, a positional argument or keyword arguments. So if your #execute method defines positional or keyword arguments, it doesn't matter.

arda(dev)* def foo(a, b)
arda(dev)*   p a, b
arda(dev)> end
=> :foo

arda(dev)* def bar(a, bar:)
arda(dev)*   p a, bar
arda(dev)> end
=> :bar

arda(dev)> foo('foo', **{ bar: 'baz' })
"foo"
{:bar=>"baz"}
=> ["foo", {:bar=>"baz"}]

arda(dev)> bar('foo', **{ bar: 'baz' })
"foo"
"baz"
=> ["foo", "baz"]

[cretz]

We don't want to build expectations on certain keyword argument presence/requirement. We want to have an unambiguous parameter signature arity in all SDKs. If you pass a hash as one of your parameters, it is one of your parameters, not sometimes one and sometimes multiple. Also the addition of a key into a hash shouldn't cause an invocation failure because you were using keyword args and hadn't yet defined that new key. If you'd like to destructure it inside your workflow that is ok, but Temporal only supports positional parameters in its serialized form and we only therefore want positional parameters in deserialized form instead of treating a trailing hash as special.