-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
bring back grpc service #5377
base: main
Are you sure you want to change the base?
bring back grpc service #5377
Conversation
* Define skeleton for GrpcAgentRuntime * Implement CloudEvent and RPC Payload serialization/marshaling
* Factor out AgentContainer for managing factory registration, instantiation, and subscription management
* Prefer AgentsAppBuilderExtensions as name, due to matching the incoming `this` type.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5377 +/- ##
=======================================
Coverage 78.08% 78.08%
=======================================
Files 158 158
Lines 9576 9576
=======================================
Hits 7477 7477
Misses 2099 2099
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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 55 out of 70 changed files in this pull request and generated no comments.
Files not reviewed (15)
- dotnet/AutoGen.sln: Language not supported
- dotnet/src/Microsoft.AutoGen/AgentHost/Microsoft.AutoGen.AgentHost.csproj: Language not supported
- dotnet/src/Microsoft.AutoGen/Agents/Microsoft.AutoGen.Agents.csproj: Language not supported
- dotnet/src/Microsoft.AutoGen/Runtime.Grpc/Services/Grpc/GrpcGatewayService.cs: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/Runtime.Grpc/Abstractions/IAgentGrain.cs: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/AgentHost/Program.cs: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/Core/AgentRuntimeExtensions.cs: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/Core.Grpc/GrpcAgentRuntime.cs: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/AgentHost/Host.cs: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/Core/TypePrefixSubscriptionAttribute.cs: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/Agents/AIAgent/InferenceAgent.cs: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/Agents/protos/agent_events.proto: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/RuntimeGateway.Grpc/Abstractions/AgentsMetadata.cs: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/RuntimeGateway.Grpc/Abstractions/AgentsRegistryState.cs: Evaluated as low risk
- dotnet/src/Microsoft.AutoGen/Agents/IOAgent/ConsoleAgent/IHandleConsole.cs: Evaluated as low risk
Comments suppressed due to low confidence (3)
dotnet/src/Microsoft.AutoGen/Agents/IOAgent/FileAgent/IHandleFileIO.cs:23
- [nitpick] Defining constants inside an interface is not a common practice. Consider moving the Route constant to a class or as part of the implementation.
const string Route = "Microsoft.AutoGen.Agents.IHandleFileIO";
dotnet/src/Microsoft.AutoGen/Agents/IOAgent/IProcessIO.cs:16
- [nitpick] Static methods in interfaces are unusual and might not be intended. Consider removing the static modifier.
static Task ProcessOutputAsync(string message) { return Task.CompletedTask; }
dotnet/src/Microsoft.AutoGen/Agents/IOAgent/IProcessIO.cs:22
- [nitpick] Static methods in interfaces are unusual and might not be intended. Consider removing the static modifier.
static Task<string> ProcessInputAsync(string message) { return Task.FromResult(message); }
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 changes in the agents package are unrelated to the gateway change it looks like
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.
that's where I moved the agents_events.proto which was used in the integration tests for xlang. not strictly required (could be rewritten to not use that - but its also a handy example of how to use custom protos with python).
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.
Sure, but the agent implementations don't belong in this PR
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 already deleted my separate PR that had them because they ended up here because of deleting the .proto which has been part of the project in the top level dir for many months.
/// <param name="topic">The topic to check subscriptions for.</param> | ||
/// <param name="eventType">The event type to check subscriptions for.</param> | ||
/// <returns>A task representing the asynchronous operation, with the list of agent IDs as the result.</returns> | ||
ValueTask<List<string>> GetSubscribedAndHandlingAgentsAsync(string topic, string eventType); |
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.
What is an event type? This shouldnt have anything to do with subscriptions.
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.
this is where you and Kosta disagreed (I'm pretty sure his implementation matches what we wrote down long ago) but in the function that part is commented out for now in order to match python.
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.
Let's align with Python on this one and remove the non-conforming implementation
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 code that runs is aligned. the event types map checks are commented out.
Restoring the grpc + Orleans server into the project
Why are these changes needed?
This is the distributed agent runtime for .NET that can manage routing messages amongst a fleet of grpc agent runtimes.
Related issue number
Checks