workflow-core icon indicating copy to clipboard operation
workflow-core copied to clipboard

Documentation for RunWorkflowSync is out of date

Open evolvedlight opened this issue 4 years ago • 4 comments

Describe the bug The documentation (in release notes) for RunWorkflowSync is out of date.

To Reproduce Try and do:

var runner = serviceProvider.GetService<ISyncWorkflowRunner>();
...
var worfklow = await runner.RunWorkflowSync("my-workflow", 1, data, TimeSpan.FromSeconds(10));

Expected behavior It to run the workflow

Additional context I'd like to contribute some documentation about how to use this, but I'd like to ensure I understand how this is supposed to work.

For example, if we had this before: var flowId = await _workflowService.StartWorkflow(Workflows.Workflow1, model);

What does this get replaced by for the sync run?

I tried: var workflow = await _syncWorkflowRunner.RunWorkflowSync(Flows.ProductIssuanceV2Flow, 2, model, "", TimeSpan.FromSeconds(10)); But that wasn't correct, and I'm not sure exactly what's expected here. For example, after this runs I don't have any subscriptions. It also took a suprisingly long time to execute, but maybe that's fine.

And I assume similar is true for the Sync version of the PublishEvent call?

Thanks very much for your help, sorry that my many dotnet 5 versions didn't all work :(

evolvedlight avatar Feb 23 '21 16:02 evolvedlight

Did you start the workflow host?

danielgerlag avatar Feb 25 '21 03:02 danielgerlag

@danielgerlag sorry, somehow missed your message. I will check that but I believe I did start the workflow host. I assume I should have started it? Or not?

evolvedlight avatar Mar 11 '21 08:03 evolvedlight

Hi @danielgerlag

Ok, so I'm now in a position that I have it all working, but I think the sync workflow runner is a little broken, but I think if you confirm my suspicions, then I can contribute a patch.

Our code wasn't working for two reasons:

  • We wait for human input to move the workflow forward (so it's runnable with without any next automatic steps
  • We expect subscriptions to be created.

So I looked at what the async runner did, and I think the sync version is missing these:

foreach(var sub in result.Subscriptions)
{
    await SubscribeEvent(sub, _persistenceStore);
}
await _persistenceStore.PersistErrors(result.Errors);

and in the while loop: add the condition workflow.NextExecution.HasValue

Would you agree with this?

evolvedlight avatar Mar 25 '21 13:03 evolvedlight

Regarding documentation, can we add RunWorkflowSync to the ReadTheDocs? I had to google, and search the issue tracker for RunWorkflowSync to find this issue to learn that theres a very short explanation on RunWorkflowSync buried in the release notes. This steps from my cancellation token not being passed to my workflow.

VictorioBerra avatar Mar 11 '22 18:03 VictorioBerra