-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Support for .NET CancellationTokens #9127
base: main
Are you sure you want to change the base?
Conversation
This reverts commit d2334d7.
There is a regression with supporting |
This would be super helpful 🙏 |
@koenbeuk is there anything remaining here or is this ready for review? Apologies for the delay |
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.
Copilot reviewed 6 out of 21 changed files in this pull request and generated 2 comments.
Files not reviewed (15)
- src/Orleans.Runtime/Cancellation/CancellableInvokableGrainExtension.cs: Evaluated as low risk
- src/Orleans.Serialization/Invocation/ICancellationRuntime.cs: Evaluated as low risk
- src/Orleans.Runtime/Hosting/DefaultSiloServices.cs: Evaluated as low risk
- src/Orleans.Runtime/Core/InsideRuntimeClient.cs: Evaluated as low risk
- src/Orleans.Runtime/Cancellation/CancellationRuntime.cs: Evaluated as low risk
- test/Grains/TestGrainInterfaces/IGenericInterfaces.cs: Evaluated as low risk
- src/Orleans.Core/Runtime/SharedCallbackData.cs: Evaluated as low risk
- test/Orleans.Serialization.UnitTests/BuiltInCodecTests.cs: Evaluated as low risk
- src/Orleans.CodeGenerator/LibraryTypes.cs: Evaluated as low risk
- src/Orleans.Core/Runtime/OutsideRuntimeClient.cs: Evaluated as low risk
- src/Orleans.CodeGenerator/SerializerGenerator.cs: Evaluated as low risk
- src/Orleans.CodeGenerator/Model/InvokableMethodDescription.cs: Evaluated as low risk
- src/Orleans.CodeGenerator/AnalyzerReleases.Unshipped.md: Evaluated as low risk
- src/Orleans.CodeGenerator/InvokableGenerator.cs: Evaluated as low risk
- test/Grains/TestGrains/GenericGrains.cs: Evaluated as low risk
Comments suppressed due to low confidence (2)
src/Orleans.Serialization/Invocation/ICancellableInvokable.cs:21
- [nitpick] The method name 'GetCancellableTokenId' is ambiguous. It should be renamed to 'GetCancellationTokenId' for better clarity.
Guid GetCancellableTokenId();
src/Orleans.Core/Runtime/CallbackData.cs:34
- The SubscribeForCancellation method introduces new behavior that should be covered by tests to ensure it works correctly under various scenarios (e.g., token already canceled, token gets canceled after registration).
public void SubscribeForCancellation(IInvokable invokable)
{ | ||
ReferenceCodec.MarkValueField(reader.Session); | ||
field.EnsureWireType(WireType.Fixed32); | ||
return new CancellationToken(reader.ReadInt32() == 1 ? true : false); |
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.
The ternary operator is redundant and should be removed.
return new CancellationToken(reader.ReadInt32() == 1 ? true : false); | |
return new CancellationToken(reader.ReadInt32() == 1); |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
This PR allows for grain methods to be cancelled using a CancellationToken. The implementation is loosely based on the existing GrainCancellationToken support that is already available.
CancellationToken
in a grain interface methodICancellableInvokableGrainExtension.CancelRemoteToken
to mark this token as canceledCancellableInvokableGrainExtension
looks up the grainsICancellationRuntime
component to mark the token as canceledICancellableInvokable
which exposes the method:GetCancellableTokenId
CancellableInvokable
sets a cancellableTokenId asGuid.NewGuid()
InvokeInner
implementation on the invokable can optionally interact with anICancellationRuntime
to track cancellation of this invokable.Once the invokable completes, its CancellationTokenSource obtained through the
ICancellationRuntime
is always CanceledFixes: #8958
Microsoft Reviewers: Open in CodeFlow