Agent Runtime Gateway Registration Consistency xLang #5755
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lokitoth
changed the title
lokitoth
force-pushed
the
dev/fix_agent_registration_consistency
branch
from
f8a494c to
fb61390
Compare
The behaviour of registration in the .NET Runtime Gateway and the Python Runtime Gateway was inconsistent: .NET allowed arbitrary numbers of clients to register the same agent type, Python did not allow duplicate agent types even in the case of re-registration by the same client. Per discussion, the decision was to meet in the middle and support re-registration but not multiplexing in the Python, and per further discussion, we decided to allow re-enabling multiplexing mode in .NET.
lokitoth
force-pushed
the
dev/fix_agent_registration_consistency
branch
from
fb61390 to
9ce8679
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5755 +/- ##
===========================================
- Coverage 70.16% 34.33% -35.83%
===========================================
Files 262 73 -189
Lines 14712 2024 -12688
Branches 243 251 +8
===========================================
- Hits 10322 695 -9627
+ Misses 4200 1131 -3069
- Partials 190 198 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rysweet
reviewed
Mar 6, 2025
| @@ -6,6 +6,15 @@ | |||
|
|
|||
| namespace Microsoft.AutoGen.RuntimeGateway.Grpc; | |||
|
|
|||
| public static class ServerCallContextExtensions | |||
There was a problem hiding this comment.
thanks for adding this - probably several other places we could compress/delete code and point to this.
| if (!_options.SupportAgentTypeMultiplexing && | ||
| _supportedAgentTypes.TryGetValue(request.Type, out List<GrpcWorkerConnection>? currentWorkers)) | ||
| { | ||
| // We should never have more than one worker unless SupportAgentTypeMultiplexing is true, so the first time we |
There was a problem hiding this comment.
Suggested change
| // We should never have more than one worker unless SupportAgentTypeMultiplexing is true, so the first time we | |
| // We should never have more than one worker with the same agent type unless SupportAgentTypeMultiplexing is true, so the first time we |
|
|
||
| await _gatewayRegistry.RegisterAgentTypeAsync(request, clientId, _reference).ConfigureAwait(true); | ||
| // If we are not supporting multiplexing, we need to ensure that the agent type is not already registered by |
There was a problem hiding this comment.
There could be other gateways. - the logic here only works fr this one - Shouldn't this be in the RegistryGrain.RegisterAgentType instead, and there you should use RegistryGrain.GetOrAddWorker() to find out if you have a connection already registered for this type?
| connection.AddSupportedType(request.Type); | ||
| _supportedAgentTypes.GetOrAdd(request.Type, _ => []).Add(connection); | ||
|
|
||
| await _gatewayRegistry.RegisterAgentTypeAsync(request, clientId, _reference).ConfigureAwait(true); |
There was a problem hiding this comment.
per above - I thik its really in this function that you want to do this...
| /// <summary> | ||
| /// Contains options for configuring the gRPC gateway. | ||
| /// </summary> | ||
| public sealed class GrpcGatewayOptions |
There was a problem hiding this comment.
should this be in its own file?
jackgerrits
approved these changes
Mar 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The behaviour of registration in the .NET Runtime Gateway and the Python Runtime Gateway was inconsistent: .NET allowed arbitrary numbers of clients to register the same agent type, Python did not allow duplicate agent types even in the case of re-registration by the same client.
Per discussion, the decision was to meet in the middle and support re-registration but not multiplexing in the Python, and per further discussion, we decided to allow re-enabling multiplexing mode in .NET.
Closes #5314