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

Fixes for deduple behavior on orchestration create #123

Merged
merged 2 commits into from
Oct 30, 2022
Merged

Conversation

cgillum
Copy link
Member

@cgillum cgillum commented Aug 18, 2022

Fixes #120

Note that this change may be breaking, depending on how exception handling logic is written. I suspect in most, if not all cases, the change from InvalidOperationException to OrchestrationAlreadyExistsException should be non-breaking, hence, I'm releasing this fix as part of a minor version update instead of a major version update.

@@ -504,14 +505,19 @@ public override async Task CreateTaskOrchestrationAsync(TaskMessage creationMess
command.Parameters.Add("@InputText", SqlDbType.VarChar).Value = startEvent.Input;
command.Parameters.Add("@StartTime", SqlDbType.DateTime2).Value = startEvent.ScheduledStartTime;

if (dedupeStatuses?.Length > 0)
{
command.Parameters.Add("@DedupeStatuses", SqlDbType.VarChar).Value = string.Join(",", dedupeStatuses);
Copy link

@mol-pensiondk mol-pensiondk Aug 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will happen if "Running" is not among the dedupeStatuses passed in? Will this clobber a running orchestration?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Will not be a problem with our use, as "Running" is always included. I'm not sure what the Azure Storage backend would do?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tested the behaviour of the Azure Storage backend. It seems that it will NOT clobber a running instance, even when "Running" is not among the dedupeStatuses. I don't get an exception (but then again, I never do with the Azure Storage backend) but the running orchestration will finish normally and not be restarted. Not so with "Pending" - a Pending orchestration will be replaced with a new one, if Pending is not among the dedupeStatuses.

@cgillum cgillum merged commit e03c30c into main Oct 30, 2022
@cgillum cgillum deleted the cgillum/dedupe branch October 30, 2022 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CreateOrchestrationInstanceAsync contract not respected
2 participants