Get rid of obsolete Sleep state and replace it with an action

[cdavernas]

What would you like to be added:

Get rid of obsolete Sleep state and replace it with an action. The state has little to no use IMO, especially since the addition of the sleepBefore/sleepAfter properties to the actionDef.

What I propose is to remove the sleepBefore/after properties and the sleep state, and replace them with an action of type sleep or delay.

...
actions:
  name: Sleep
  delay: PT30M
...

Why is this needed:

What the sleep state achieves can be done using the sleepBefore/after properties, which IMO are very wierd in the context of an action. As a matter of fact, it should not be the responsibility or in the domain of the action to delay its own processing or its return. A better way is IMO to achieve that feature with an EXPLICIT action, which will therefore be visible in visualization tools, making the spec more ubiquitous, clean and concise.

[tsurdilo]

Currently actions require one of the "refs" to point to a function definition: https://github.com/serverlessworkflow/specification/blob/main/schema/workflow.json#L486
How would you suggest this for sleep, to have a "sleep" function type maybe but that seems kinda not natural. Let me know please.

[cdavernas]

@tsurdilo I'm nowhere recommending putting that as a new function type. I'm recommending putting it as an action type.

So far, we implicitly have 3 action types:

• Subflow
• Function
• Event

I'm recommending adding:

• Sleep/Delay

[fjtirado]

Do we have a use case for sleep? Im asking because Ill remove sleeps altogether. We should wait for something, not a random amount of time.

[ricardozanini]

I'm also curious to hear a use case for sleep.

[fjtirado]

Assuming we have to wait a random amount of time after an action has been executed and before another action is executed, I would add the sleep as an optional (and discouraged) property of the "weak" action itself.
In my opinion, waits are indication of a flaw in the design of a particular action, so lets keep them isolated in the place where they might be used as last resort (which in my opinion is the flawed action)

[tsurdilo]

dont diss sleep so much . it has very valid use its rly important for testing as well as waiting on certain time to trigger actions / continue. it should not be discouraged to be used

[tsurdilo]

sleep in actions are important for testing . in a test you could have a fake action that just returns and use sleep to simulate diff exec times and test timeouts .

[fjtirado]

yes, but you are testing sleeps associated to actions, isnt it?

[tsurdilo]

no, the before after can be used to simulate action exec time and action timeouts. the sleep state is useful yea to test wf exec timeout and state timeout but also has valid business logic use cases

[cdavernas]

@fjtirado IMO, sleep can be usefull, but they should be an action, not an action prop.

Imagine a more real-life scenario than just "emulating" network delay, as explained by @tsurdilo: Long polling on a remote API. You would loop while a condition is true and make sure to wait an explicit amount of time before and/or after the execution of the polling request.

[fjtirado]

@cdavernas The way I envision that scenario is that the action you use to perform to the polling has the property to wait after it is executed (rather than adding a "fake" action there to wait)

[tsurdilo]

for polling there are couple ways, one is using sleep as described, another is polling via retries and backoff coefficient third is polling inside the action function itself

[tsurdilo]

i think sleep state feels more naturan in some cases than sleep action which ppl associate with actual invocation of things..but can be argued as its preference. only problem is timeouts as for sleep action its bound by action timeout but who knows thats fine

[ricardozanini]

Okay, I can see us cases for testing, but not for pooling. If you guys wanna go ahead with this, I think an action like what Charles was proposing might have more sense. If I see a workflow definition with sleep without a testing context, I'd assume that's a workaround.

[tsurdilo]

but not for pooling

for polling with sleep state think of simple operationState(perform action)->switchState(check result)->sleepState(sleep for x time)->loop back to operationState
thats how i think you would do it in bpmn too

its a valid use case imo but all i said was you can also poll via action retries too

[ricardozanini]

That makes sense, but I would implement it differently unless I'd need to directly express the sleep stage in the definition.

Yeah, it makes it a valid use case.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[fjtirado]

@cdavernas As you propose, I think we can remove sleep state and rely on sleep before and after function execution.
I do not think there is a use case to sleep before or after doing nothing.