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

Add a new suspended workflow status phase #1024

Open
cdavernas opened this issue Oct 25, 2024 · 8 comments · May be fixed by #1054
Open

Add a new suspended workflow status phase #1024

cdavernas opened this issue Oct 25, 2024 · 8 comments · May be fixed by #1054
Assignees
Labels
area: spec Changes in the Specification change: feature New feature or request. Impacts in a minor version change
Milestone

Comments

@cdavernas
Copy link
Member

cdavernas commented Oct 25, 2024

What would you like to be added:

Reintroduce the suspended workflow status phase

Why is this needed:

Adds a status phase that clearly describes that the workflow has been manually and explicitly suspended by a user, as opposed to waiting, which describes that the workflow is waiting for event(s) to be correlated.

Additional notes:

The suspended status phase was initialy part of the DSL, but was removed following (very valid) observations from @fjtirado, who considered that waiting could actually represent both phases.

However, following the implementation of the suspend feature in Synapse, we realized that users could not differenciate flows that were explicitly suspended from ones that were being correlated, thus the feature request.

@cdavernas cdavernas added change: feature New feature or request. Impacts in a minor version change area: spec Changes in the Specification labels Oct 25, 2024
@cdavernas cdavernas added this to the v1.0.0 milestone Oct 25, 2024
@cdavernas cdavernas changed the title Add a new suspended workflow instance status phase Add a new suspended workflow status phase Oct 25, 2024
@cdavernas cdavernas self-assigned this Oct 25, 2024
@fjtirado
Copy link
Collaborator

ok, so waiting is a "natural" status, the workflow reach it as part of its execution while suspended is "forced" one, explicitly set by the user.
My only concern is that suspending a workflow should not necessarily be implemented by all implementors. I mean, I can foresee an implementation that does not provide ways to suspend workflow execution and its still a compliant one, wdyt?

@cdavernas
Copy link
Member Author

cdavernas commented Oct 25, 2024

ok, so waiting is a "natural" status, the workflow reach it as part of its execution while suspended is "forced" one, explicitly set by the user.

That's my understanding of it, yes.

I mean, I can foresee an implementation that does not provide ways to suspend workflow execution and its still a compliant one, wdyt?

@fjtirado Very valid point indeed. I personally don't see the requirement of enforcing it. That status phase is IMO a nice to have in case one implements the related feature. Furthermore, adding it would allows us to exhaustively describe all the possible phases of a workflow, regardless of what has been implemented or not.

@fjtirado
Copy link
Collaborator

fjtirado commented Oct 25, 2024

As an alternative, I think we can broadly understand waiting as waiting for something (not just events), so it will also included the state "waiting for the user to eventually resume the workflow he temporalily stopped for some reason" and then let implementors provides "substates" or "states additional description" freely. Because there might be implementations that might want to differentiate between "waiting for event" or "waiting for rest response" and this will allow them to do that.

@fjtirado
Copy link
Collaborator

fjtirado commented Oct 25, 2024

Or following a similar idea, add an additional actor field, which will include the "initiator", human or machine ;), that let the workflow to reach the current state
So in the suspend case, state will be waiting and actor will be user name
In the waiting for event case., state will be waiting and actor will be event type

@fjtirado
Copy link
Collaborator

fjtirado commented Oct 25, 2024

The reason Im proposing alternatives is that once we differentiate between two ways of waiting (waiting to resume after user intervention, suspended, and waiting for an event to arrive as defined in the file, current waiting), more will eventually come up (not necessarily applicable to all implementations, as is the case for suspended), but I maybe wrong and we will just have those two only.

Copy link

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.

@cdavernas
Copy link
Member Author

@fjtirado Can we start by just adding the suspended one, for the time being?

@fjtirado
Copy link
Collaborator

fjtirado commented Jan 8, 2025

ok, so waiting will be the status for workflows listening or waiting and suspended will be the status for workflows that has been paused by users.
However, after partially implementing the spec, I wonder if we should indicate that suspension will happen after the current task has finished. This means that if a workflow is already waiting, the suspend request should be ignored. This also means that if a task is performing a rest call (which is taking a while to respond), the output filter over that response will be eventually applied and then the workflow will pause before executing the next task.
Im not sure all of this should be specified or leave to the implementor.
It might sound weird that spec enter so much into implementation details, but the fact that we standarize suspended means that the implementation should be designed with the possibility that a suspended request might be received (this rule out purely sync implementations or at least implies purely sync implementation has to cope with that). But we already has pending, which implies that the instance might exist before being executed (which depending on the implementation might not make sense, I mean creating and starting the instance might happen atomically)

@cdavernas cdavernas linked a pull request Jan 9, 2025 that will close this issue
8 tasks
@cdavernas cdavernas linked a pull request Jan 9, 2025 that will close this issue
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: spec Changes in the Specification change: feature New feature or request. Impacts in a minor version change
Projects
Status: Backlog
2 participants