-
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
Dotnet Grpc worker implementation #5245
base: main
Are you sure you want to change the base?
Conversation
1b9ad7a
to
ff2b423
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5245 +/- ##
=======================================
Coverage 76.02% 76.02%
=======================================
Files 157 157
Lines 9491 9491
=======================================
Hits 7216 7216
Misses 2275 2275
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.
I think that most of the changes to the server side aren't necessary - I would just bring back the files from the old core that it was depending upon to start with and get the client talking to it. It was working xlang e2e (ie with the python client) on Monday am
* 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.
100e9e0
to
7c3c934
Compare
Yep - scoped this back to the worker only |
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.
It looks like GrpcMessageRouter and GrpcAgentWorker have a bunch of duplicate - assume not intentionally.
I think we should add tests for this before we add it back in - I'm happy to take that on.
No description provided.