[Workflow] Add options parameter to workflow start method

[cgillum]

Describe the feature

This proposal applies to the DaprWorkflowClient.ScheduleNewWorkflowAsync method, which is specific to the Dapr Workflow engine. It doesn't apply to DaprClient.StartWorkflowAsync, which is the "building block" API that is engine agnostic.

The current signature looks like this:

public Task<string> ScheduleNewWorkflowAsync(
    string name,
    string? instanceId = null,
    object? input = null,
    DateTime? startTime = null)
{
    // ...
}

As an aside, Dapr Workflow doesn't currently support the startTime parameter, so this needs to be removed

However, it should ideally be changed to better align with Microsoft's Azure SDK guidelines for clients since those guidelines represent general best practices and many .NET developers will find them familiar.

Since this method could be considered complex, or could turn into a complex method, it should be changed to look something like this:

public Task<string> ScheduleNewWorkflowAsync(
    string name,
    ScheduleWorkflowOptions? options = default)
{
    // ...
}

public class ScheduleNewWorkflowOptions
{
    public string? InstanceId { get; set; }
    public object? Input { get; set; }
    // start time
    // ID reuse/conflict policy
    // workflow tags
    // etc.
}

This should be done for the beta release since it's a breaking change and likely represents the path forward.

Release Note

RELEASE NOTE: UPDATE New (breaking) method signature for starting workflows

[olitomlinson]

My 2c

public class ScheduleNewWorkflowOptions
{
    // start time
    public ReusePolicy ReusePolicy { get; set; }
    // workflow tags
    // etc.
}

public class ReusePolicy
{
    public WorkflowState[] PermittedStates { get; set; }
    public OnPolicyViolation PolicyViolationAction { get; set; }
}

public enum WorkflowState
{
   Pending,
   Running,
   Completed,
   Failed,
   Paused,
   Terminated
}

public enum PolicyViolationAction
{
   Silent | Skip | Supress,
   Throw
}

---

The default behaviour when ScheduleNewWorkflowOptions has not been specified would be the equivalent of :

new ScheduleNewWorkflowOptions
{
    ReusePolicy = new ReusePolicy 
    {
        // No reuse, therefor maximum safety against accidentally rerunning a 
        // workflow and performing potentially duplicate / undesirable side-effects
        PermittedStates = {},
        
        // Maximum awareness to devs/operators that re-use is not occurring by 
        // throwing, they can choose to suppress if this is noisy
        OnPolicyViolation = PolicyViolationAction.Throw
    }
}

---

The Exception thrown by PolicyViolationAction.Throw can be polite and hint to the dev/operator that supression options are available and the allowed Reuse states can be specified.

InvalidOperationException : Reuse of this workflow instance ID is not permitted. Specify a custom 'ReusePolicy' to alter the conditions that reuse is permitted, and how policy violations are handled.

---

For @Clintsigner use-case, the option would be :

   daprWorkflowClient.ScheduleNewWorkflowAsync(
      name : ...,
      instanceId : ...,
      options : new ScheduleNewWorkflowOptions {
         reusePolicy : new ReusePolicy {  
            PermittedStates = { WorkflowState.Running, ... }
      }});

This would permit an existing running/etc workflow to be replaced by another

[clintsinger]

I like the idea of moving the options to an actual options class. Definitely allows for future growth without breaking changes. @olitomlinson proposal of a re-use policy also looks good. The only thing I'd change at this time is change the PermittedStates to be an enum flags rather than an array and possibly rename it to Conditions.

 [Flags]
 public enum WorkflowState
 {
      None = 0
      Pending = 1,
      Running = 2,
      Completed = 4,
      Failed = 8,
      Paused = 16,
      Terminated = 32,
      All = Pending | Running | Completed | Failed | Paused | Terminated
}

By using enum flags the conditions can be merged and defaults can be included, such as an All option.

daprWorkflowClient.ScheduleNewWorkflowAsync(
      name : ...,
      instanceId : ...,
      reusePolicy : new ReusePolicy {  
         Conditions = WorkflowState.All
      });

The user can also use it in a fine grain manner by cherry picking specific conditions .

daprWorkflowClient.ScheduleNewWorkflowAsync(
      name : ...,
      instanceId : ...,
      reusePolicy : new ReusePolicy {  
         Conditions = WorkflowState.Completed | WorkflowState.Failed
      });

This one wouldn't allow re-use and would be the default condition.

daprWorkflowClient.ScheduleNewWorkflowAsync(
      name : ...,
      instanceId : ...,
      reusePolicy : new ReusePolicy {  
         Conditions = WorkflowState.None
      });

[clintsinger]

As to the DelayStart option, I think it would be great to still include it as part of the scheduling rather than relying on workflows always needing the CreateTimer internally.

Adding it would close dapr/dapr#6450

[olitomlinson]

@clintsinger

The only thing I'd change at this time is change the PermittedStates to be an enum flags rather than an array and possibly rename it to Conditions.

Good suggestions, I'm onboard.

[olitomlinson]

As to the DelayStart option, I think it would be great to still include it as part of the scheduling rather than relying on workflows always needing the CreateTimer internally.

I agree. Tucking the StartTimeUtc property into the options would make sense, if thats possible.

   daprWorkflowClient.ScheduleNewWorkflowAsync(
      name : ...,
      instanceId : ...,
      options : new ScheduleNewWorkflowOptions {
         StartTimeUtc : DateTime.UtcNow().AddMinutes(5),
         ReusePolicy : new ReusePolicy {  
           ...
         }
      });

My only question here is at the point of performing the above operation, the runtime would have to 'claim' that particular Workflow ID (or not, and throw an exception etc) -- So what would the runtime status of the workflow be during the 5 minute window before the Workflow actually starts? scheduled ?

[clintsinger]

Perhaps instead of DelayStart being a DateTime it could be a TimeSpan to match what is in CreateTimer?

As for the state I think the Pending state would be sufficient. Pending could also be renamed to Scheduled.

[clintsinger]

I'm not sure where I got the DelayStart name as the startTime parameter, but either way I still think we should keep it but have it act as if there was a CreateTimer with a TimeSpan.

Probably also need a similar option for child workflows, or continue as new workflows.