-
Notifications
You must be signed in to change notification settings - Fork 52
Add version to TaskName #416
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| namespace Microsoft.DurableTask; | ||
|
|
||
| /// <summary> | ||
| /// The version of a durable task. | ||
| /// </summary> | ||
| public readonly struct TaskVersion : IEquatable<TaskVersion> | ||
| { | ||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="TaskVersion"/> struct. | ||
| /// </summary> | ||
| /// <param name="version">The version of the task. Providing <c>null</c> will result in the default struct.</param> | ||
| public TaskVersion(string version) | ||
| { | ||
| if (version == null) | ||
| { | ||
| this.Version = null!; | ||
| } | ||
| else | ||
| { | ||
| this.Version = version; | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the version of a task. | ||
| /// </summary> | ||
| public string Version { get; } | ||
|
|
||
| /// <summary> | ||
| /// Implicitly converts a <see cref="TaskVersion"/> into a <see cref="string"/> of the <see cref="Version"/> property value. | ||
| /// </summary> | ||
| /// <param name="value">The <see cref="TaskVersion"/> to be converted into a <see cref="string"/>.</param> | ||
| public static implicit operator string(TaskVersion value) => value.Version; | ||
|
|
||
| /// <summary> | ||
| /// Implicitly converts a <see cref="string"/> into a <see cref="TaskVersion"/>. | ||
| /// </summary> | ||
| /// <param name="value">The <see cref="string"/> to convert into a <see cref="TaskVersion"/>.</param> | ||
| public static implicit operator TaskVersion(string value) => new TaskVersion(value); | ||
|
|
||
| /// <summary> | ||
| /// Compares two <see cref="TaskVersion"/> structs for equality. | ||
| /// </summary> | ||
| /// <param name="a">The first <see cref="TaskVersion"/> to compare.</param> | ||
| /// <param name="b">The second <see cref="TaskVersion"/> to compare.</param> | ||
| /// <returns><c>true</c> if the two <see cref="TaskVersion"/> objects are equal; otherwise <c>false</c>.</returns> | ||
| public static bool operator ==(TaskVersion a, TaskVersion b) | ||
| { | ||
| return a.Equals(b); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Compares two <see cref="TaskVersion"/> structs for inequality. | ||
| /// </summary> | ||
| /// <param name="a">The first <see cref="TaskVersion"/> to compare.</param> | ||
| /// <param name="b">The second <see cref="TaskVersion"/> to compare.</param> | ||
| /// <returns><c>false</c> if the two <see cref="TaskVersion"/> objects are equal; otherwise <c>true</c>.</returns> | ||
| public static bool operator !=(TaskVersion a, TaskVersion b) | ||
| { | ||
| return !a.Equals(b); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets a value indicating whether to <see cref="TaskVersion"/> objects | ||
| /// are equal using value semantics. | ||
| /// </summary> | ||
| /// <param name="other">The other <see cref="TaskVersion"/> to compare to.</param> | ||
| /// <returns><c>true</c> if the two <see cref="TaskVersion"/> are equal using value semantics; otherwise <c>false</c>.</returns> | ||
| public bool Equals(TaskVersion other) | ||
| { | ||
| return string.Equals(this.Version, other.Version, StringComparison.OrdinalIgnoreCase); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets a value indicating whether to <see cref="TaskVersion"/> objects | ||
| /// are equal using value semantics. | ||
| /// </summary> | ||
| /// <param name="obj">The other object to compare to.</param> | ||
| /// <returns><c>true</c> if the two objects are equal using value semantics; otherwise <c>false</c>.</returns> | ||
| public override bool Equals(object? obj) | ||
| { | ||
| if (obj is not TaskVersion other) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| return this.Equals(other); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Calculates a hash code value for the current <see cref="TaskVersion"/> instance. | ||
| /// </summary> | ||
| /// <returns>A 32-bit hash code value.</returns> | ||
| public override int GetHashCode() | ||
| { | ||
| return StringComparer.OrdinalIgnoreCase.GetHashCode(this.Version); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -139,6 +139,7 @@ | |
| try | ||
| { | ||
| // TODO: Cancellation (https://github.com/microsoft/durabletask-dotnet/issues/7) | ||
| #pragma warning disable 0618 | ||
| if (options?.Retry?.Policy is RetryPolicy policy) | ||
| { | ||
| return await this.innerContext.ScheduleWithRetry<T>( | ||
|
|
@@ -165,6 +166,7 @@ | |
| // Hide the core DTFx types and instead use our own | ||
| throw new TaskFailedException(name, e.ScheduleId, e); | ||
| } | ||
| #pragma warning restore 0618 | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
|
|
@@ -177,10 +179,12 @@ | |
| static string? GetInstanceId(TaskOptions? options) | ||
| => options is SubOrchestrationOptions derived ? derived.InstanceId : null; | ||
| string instanceId = GetInstanceId(options) ?? this.NewGuid().ToString("N"); | ||
| string defaultVersion = this.invocationContext.Options?.Versioning?.DefaultVersion ?? string.Empty; | ||
| string version = options is SubOrchestrationOptions subOptions ? subOptions.Version : defaultVersion; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just want to leave a comment regarding what we discussed offline here. I don't think it is a good idea to conflate worker version and orchestration version. I see these as separate concerns. Worker version is purely for routing and supporting rolling updates. Having worker version apply as a default to orchestration version unnecessarily complicates that scenario. Now customers need to think about how their individual orchestrations version in relation to their worker version as a whole. Which one takes precedence? How does a worker version less than an orchestration version behave? What if they aren't comparable at all? All of these questions will need to be considered, answered, and documented if we keep these intertangled. Whereas if we just keep them entirely separate then answering all of that becomes much simpler.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I appreciate the comment and the conversation we had offline about this! I'll make sure that this is all clearly documented/explained.
The default version only really applies to sub-orchestrations from a Worker point of view. It's matching the client side's default version. In my opinion, the worker shouldn't be starting sub-orchestrations without the client but since it does, we provide a means of setting it. For functions apps, this is the same In regard to what takes precedence, it's always the version set by the options when scheduling an orchestration. The worker version is only used for the matching/accepting of an orchestration. The way they interact is set with the version matching strategy and the version failure strategy. The docs aren't published to learn yet as we wanted support across the SDKs and DTFx first, but this sample readme talks about the interaction: Orchestration versioning sample.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this was covered in the offline discussion, but it's probably best to not think of or describe this as a "worker version", but rather as a "default orchestration version" which happens to be something that can be configured at a client and/or worker level. |
||
|
|
||
| Check.NotEntity(this.invocationContext.Options.EnableEntitySupport, instanceId); | ||
|
|
||
| // if this orchestration uses entities, first validate that the suborchsestration call is allowed in the current context | ||
| // if this orchestration uses entities, first validate that the suborchestration call is allowed in the current context | ||
| if (this.entityFeature != null && !this.entityFeature.EntityContext.ValidateSuborchestrationTransition(out string? errorMsg)) | ||
| { | ||
| throw new InvalidOperationException(errorMsg); | ||
|
|
@@ -192,7 +196,7 @@ | |
| { | ||
| return await this.innerContext.CreateSubOrchestrationInstanceWithRetry<TResult>( | ||
| orchestratorName.Name, | ||
| orchestratorName.Version, | ||
| version, | ||
| instanceId, | ||
| policy.ToDurableTaskCoreRetryOptions(), | ||
| input); | ||
|
|
@@ -202,7 +206,7 @@ | |
| return await this.InvokeWithCustomRetryHandler( | ||
| () => this.innerContext.CreateSubOrchestrationInstance<TResult>( | ||
| orchestratorName.Name, | ||
| orchestratorName.Version, | ||
| version, | ||
| instanceId, | ||
| input), | ||
| orchestratorName.Name, | ||
|
|
@@ -213,7 +217,7 @@ | |
| { | ||
| return await this.innerContext.CreateSubOrchestrationInstance<TResult>( | ||
| orchestratorName.Name, | ||
| orchestratorName.Version, | ||
| version, | ||
| instanceId, | ||
| input); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we evaluated versioning for
TaskActivity? DurableTask.Core has it. I personally haven't used it much myself, unlike orchestration versioning which I used extensively. Still wondering if it is planned?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also haven't used it much, the initial thought here is that differently versioned orchestrations can have the different activities and different version activities in the same orchestration were less likely.
That being said, we've left the door open for it, so if it's a requested feature or something we agree to as a team we can still implement it down the road.