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.

Sign up for GitHub

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 versioning support(phase1) #2799

Open
wants to merge 6 commits into
base: v2.x
Choose a base branch
from
Open

Add versioning support(phase1) #2799

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
49b309b
Add override StartNewAsync() method to set instanceVersion
skyao Apr 22, 2024
acfeaaa
Update StartNewAsync() method to set instanceVersion correctly to cal…
skyao Apr 22, 2024
70e56a0
Update to call IDurableOrchestrationClient.StartNewAsync() method wit…
skyao Apr 22, 2024
d5b1b15
Auto update for new added override StartNewAsync() method with instan…
skyao Apr 22, 2024
f829e9d
Implemented new added abstract method to get instance version
skyao Apr 22, 2024
76f398d
Merge branch 'Azure:dev' into versioning-phase1
skyao Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/WebJobs.Extensions.DurableTask/ContextImplementations/DurableClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ async Task<IActionResult> IDurableOrchestrationClient.WaitForCompletionOrCreateC
}

/// <inheritdoc />
async Task<string> IDurableOrchestrationClient.StartNewAsync<T>(string orchestratorFunctionName, string instanceId, T input)
async Task<string> IDurableOrchestrationClient.StartNewAsync<T>(string orchestratorFunctionName, string instanceId, string instanceVersion, T input)
{
if (this.ClientReferencesCurrentApp(this))
{
Expand All @@ -202,7 +202,7 @@ async Task<string> IDurableOrchestrationClient.StartNewAsync<T>(string orchestra

OrchestrationStatus[] dedupeStatuses = this.GetStatusesNotToOverride();
Task<OrchestrationInstance> createTask = this.client.CreateOrchestrationInstanceAsync(
orchestratorFunctionName, DefaultVersion, instanceId, input, null, dedupeStatuses);
orchestratorFunctionName, instanceVersion, instanceId, input, null, dedupeStatuses);

this.traceHelper.FunctionScheduled(
this.TaskHubName,
Expand Down Expand Up @@ -1139,6 +1139,12 @@ Task<string> IDurableOrchestrationClient.StartNewAsync<T>(string orchestratorFun
return ((IDurableOrchestrationClient)this).StartNewAsync<T>(orchestratorFunctionName, string.Empty, input);
}

/// <inheritdoc/>
Task<string> IDurableOrchestrationClient.StartNewAsync<T>(string orchestratorFunctionName, string instanceId, T input)
{
return ((IDurableOrchestrationClient)this).StartNewAsync<T>(orchestratorFunctionName, instanceId, string.Empty, input);
}

async Task<string> IDurableOrchestrationClient.RestartAsync(string instanceId, bool restartWithNewInstanceId)
{
DurableOrchestrationStatus status = await ((IDurableOrchestrationClient)this).GetStatusAsync(instanceId, showHistory: false, showHistoryOutput: false, showInput: true);
Expand Down
19 changes: 19 additions & 0 deletions src/WebJobs.Extensions.DurableTask/ContextInterfaces/IDurableOrchestrationClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,25 @@ Task<string> StartNewAsync<T>(
/// </exception>
Task<string> StartNewAsync<T>(string orchestratorFunctionName, string instanceId, T input);

/// <summary>
/// Starts a new instance of the specified orchestrator function.
/// </summary>
/// <remarks>
/// If an orchestration instance with the specified ID already exists, the existing instance
/// will be silently replaced by this new instance.
/// </remarks>
/// <param name="orchestratorFunctionName">The name of the orchestrator function to start.</param>
/// <param name="instanceId">The ID to use for the new orchestration instance.</param>
/// <param name="instanceVersion">The version to use for the new orchestration instance.</param>
/// <param name="input">JSON-serializable input value for the orchestrator function.</param>
/// <typeparam name="T">The type of the input value for the orchestrator function.</typeparam>
/// <returns>A task that completes when the orchestration is started. The task contains the instance id of the started
/// orchestratation instance.</returns>
/// <exception cref="ArgumentException">
/// The specified function does not exist, is disabled, or is not an orchestrator function.
/// </exception>
Task<string> StartNewAsync<T>(string orchestratorFunctionName, string instanceId, string instanceVersion, T input);
Copy link
Member

Choose a reason for hiding this comment

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

Adding a new method without a default implementation to an existing interface is a breaking change, which we can't allow. We will need to find a way to add this overload without introducing a breaking change.

Copy link
Author

Choose a reason for hiding this comment

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

Can we change this method to a default method of interface, Like:

        Task<string> StartNewAsync<T>(string orchestratorFunctionName, string instanceId, string instanceVersion, T input) {
          return StartNewAsync(orchestratorFunctionName, instanceId, input);
        }

But this need runtime to support default interface implementation:

C:\Users\sky\work\code\durabletask-fork2\azure-functions-durable-extension\src\WebJobs.Extensions.DurableTask\ContextInterfaces\IDurableOrchestrationClient.cs(182,22): erro 
r CS8701: Target runtime doesn't support default interface implementation. [C:\Users\sky\work\code\durabletask-fork2\azure-functions-durable-extension\src\WebJobs.Extension 
s.DurableTask\WebJobs.Extensions.DurableTask.csproj::TargetFramework=netstandard2.0]

Copy link
Author

Choose a reason for hiding this comment

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

Another solution is trying to reuse orchestratorFunctionName, we can add version to this parameter with a predefined seperator(using @ as example, we can make desicion later if we accept this solution) :

orchestratorFunctionName="HelloOrchestrator"
orchestratorFunctionName="[email protected]"

In the implementation, we can try to split orchestratorFunctionName to get real orchestratorFunctionName and optional version:

orchestratorFunctionName="HelloOrchestrator" --> orchestratorFunctionName="HelloOrchestrator", version=""
orchestratorFunctionName="[email protected]" --> orchestratorFunctionName="HelloOrchestrator", version="1.2.0"

This solution should work fine without breaking change, but it's just a bit ugly.

Copy link
Contributor

@jviau jviau Apr 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@ is already used for entity support and has some existing semantics to it. And if we do go with some special character separator, we would have to consider the impact to existing instance IDs that might already contain that character.

Adding support here is challenging without a breaking change. The simplest route would be a new interface IDurableOrchestrationClient2 and we add it there.

There is also another option: don't add support for this for in-proc. Add it to out of proc only.

Copy link
Author

Choose a reason for hiding this comment

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

we can change @ to something else like ||.

This combine of orchestratorFunctionName and version will only be used to call this StartNewAsync() method:

  • Before calling StartNewAsync, there are two separated variables: orchestratorFunctionName and version, we just combine them as "orchestratorFunctionNam||version" to pass them into StartNewAsync() method with one orchestratorFunctionName paremeter.

  • In StartNewAsync() method, we will split orchestratorFunctionName paremeter by || at first and get back real orchestratorFunctionName and version, then "orchestratorFunctionName||version" won't use any more.

Copy link
Author

Choose a reason for hiding this comment

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

@cgillum @jviau would you please have a look at this PR and make a decision to make it work without breaking changes.

Copy link
Member

Choose a reason for hiding this comment

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

Let's go with @jviau's suggestion, and NOT add support for versioning in the WebJobs APIs. These WebJobs APIs are already moving towards deprecation. Let's instead focus on just the APIs defined in microsoft/durabletask-dotnet. If you agree, then we can close this PR.

Copy link
Author

Choose a reason for hiding this comment

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

I agree.

Yet This PR is used by me in the updated quickstart to show how versioning works in durable-functions, as well as:

https://learn.microsoft.com/en-us/azure/azure-functions/durable/durable-functions-create-first-csharp?pivots=code-editor-vscode#add-functions-to-the-app

Without this PR, I can't how an end-to-end process to do versioning.


/// <summary>
/// Sends an event notification message to a waiting orchestration instance.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/WebJobs.Extensions.DurableTask/LocalGrpcListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public override Task<Empty> Hello(Empty request, ServerCallContext context)
try
{
string instanceId = await this.GetClient(context).StartNewAsync(
request.Name, request.InstanceId, Raw(request.Input));
request.Name, request.InstanceId, request.Version, Raw(request.Input));
return new P.CreateInstanceResponse
{
InstanceId = instanceId,
Expand Down
24 changes: 23 additions & 1 deletion src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions src/Worker.Extensions.DurableTask/FunctionsOrchestrationContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public FunctionsOrchestrationContext(TaskOrchestrationContext innerContext, Func

public override string InstanceId => this.innerContext.InstanceId;

public override string InstanceVersion => this.innerContext.InstanceVersion;

public override DateTime CurrentUtcDateTime => this.innerContext.CurrentUtcDateTime;

public override bool IsReplaying => this.innerContext.IsReplaying;
Expand Down