CreateOrchestrationInstanceAsync contract not respected

[mol-pensiondk]

De-dupe statuses passed to CreateOrchestrationInstanceAsync are not respected by the MSSQL backend. Also, the exception type thrown when an instance is not created due to duplicate ID is wrong.

In the following call, de-dupe statuses are explicitly given and include stauses beyond Pending and Running.

return await _taskHubClient.CreateOrchestrationInstanceAsync(
    "DurableTaskTest.TestOrchestration",
    null, // version
    oId,
    oInput,
    null, // tags
    new OrchestrationStatus[] {
        OrchestrationStatus.Running,
        OrchestrationStatus.Completed,
        OrchestrationStatus.ContinuedAsNew,
        OrchestrationStatus.Pending });

I use this to implement idempotency, allowing only restart of orchestrations which have failed. The Azure Storage backend works as expected and does not restart an already completed orchestration. The MSSQL back-end however does not respect the de-dupe parameter (indeed, it has a hardcoded list consisting of Pending and Running in the CreateInstance stored procedure).

Also, the exception thrown when a duplicate is detected is InvalidOperationException. I would have expected DurableTask.Core.Exceptions.OrchestrationAlreadyExistsException

[cgillum]

Through code inspection, I confirmed that you're right on both issues.

One thing worth noting is that the Azure Storage backend also throws InvalidOperationException when a duplicate instance is created. I agree that changing this to OrchestrationAlreadyExistsException is the right thing to do. The good news is that OrchestrationAlreadyExistsException derives from InvalidOperationException, so it seems like we can make this change in a mostly non-breaking way.

[mol-pensiondk]

Wow, that's fast you have a fix in the pipeline!

Regarding the Azure Storage backend throwning InvalidOperationException: Actually I can't get it to throw anything. It always returns an OrchestrationInstance, regardless of whether the orchestration was created or not. It will have a non-existing ExecutionId, so is clearly the handle for the orchestartion that would have been created, if not for the de-dupe check.