Extended sessions for entities in .NET isolated #507
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR extends the extended sessions feature to entities in .NET isolated, bringing entities to feature parity with orchestrations. The implementation refactors shared logic between GrpcOrchestrationRunner and GrpcEntityRunner into a new utility class and applies the same extended session caching pattern to entities.
Key Changes:
- Introduced
GrpcInstanceRunnerUtilsto share request property parsing and cache initialization logic - Added extended sessions support to
GrpcEntityRunnerwith state caching capabilities - Updated parameter naming from
IncludePastEventstoIncludeStatethroughout tests for consistency
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Worker/Grpc/GrpcInstanceRunnerUtils.cs | New utility class containing shared logic for parsing request properties and initializing extended sessions cache |
| src/Worker/Grpc/GrpcEntityRunner.cs | Added new overload with extended sessions support and implemented entity state caching logic |
| src/Worker/Grpc/GrpcOrchestrationRunner.cs | Refactored to use shared utility for request property parsing and cache initialization |
| test/Worker/Grpc.Tests/GrpcEntityRunnerTests.cs | New comprehensive test suite covering extended sessions scenarios for entities |
| test/Worker/Grpc.Tests/GrpcOrchestrationRunnerTests.cs | Updated tests to use IncludeState parameter naming and improved variable naming consistency |
Comments suppressed due to low confidence (1)
src/Worker/Grpc/GrpcOrchestrationRunner.cs:210
- Variable extendedSessions may be null at this access as suggested by this null check.
Variable extendedSessions may be null at this access as suggested by this null check.
extendedSessions.Set<ExtendedSessionState>(
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
No blockers - just a few comment suggestions.
| } | ||
| else | ||
| { | ||
| extendedSessions?.Remove(request.InstanceId); |
There was a problem hiding this comment.
In what situations would we successfully remove something from this cache here?
There was a problem hiding this comment.
This is to handle the situation where the caller of this method has specifically indicated that the extended session has ended (so the extended sessions cache is non-null, but isExtendedSession is false. We want to honor the termination of the extended session and remove the entity state from the cache here). The other path that could get us here is if extended sessions are simply not enabled, in which case the cache will be null, hence the need for the ?
| string requestString = Convert.ToBase64String(requestBytes); | ||
| string stringResponse = GrpcOrchestrationRunner.LoadAndRun(requestString, new SimpleOrchestrator(), extendedSessions); | ||
| Protobuf.OrchestratorResponse response = Protobuf.OrchestratorResponse.Parser.ParseFrom(Convert.FromBase64String(stringResponse)); | ||
| string requestString = Convert.ToBase64String(requestBytes); |
There was a problem hiding this comment.
Inconsistent spacing: there are two spaces after the equals sign. Should be a single space for consistency with the rest of the codebase.
| string requestString = Convert.ToBase64String(requestBytes); | |
| string requestString = Convert.ToBase64String(requestBytes); |
| entityRequest.Properties.Add(new MapField<string, Value>() { | ||
| { "IncludeState", Value.ForBool(false) }}); | ||
| byte[] requestBytes = entityRequest.ToByteArray(); | ||
| string requestString = Convert.ToBase64String(requestBytes); |
There was a problem hiding this comment.
Inconsistent spacing: there are two spaces after the equals sign. Should be a single space for consistency with the rest of the codebase.
| string requestString = Convert.ToBase64String(requestBytes); | |
| string requestString = Convert.ToBase64String(requestBytes); |
This PR introduces the extended sessions feature for entities in .NET isolated. It essentially copies what was done to enable extended sessions for orchestrations in the
GrpcOrchestrationRunnerinto theGrpcEntityRunner, and introduces a new class for shared logic between the two.