-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
.NET combine Handle and CallHandler #4386
base: main
Are you sure you want to change the base?
Conversation
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.
sorry I posted these comments a while back and today while using gh pr from cli I realized that they didn't post fully (submit review) - they were still "pending". anyway I think this is all supplanted by the new PRs
Task<RpcResponse> HandleRequest(RpcRequest request); | ||
void ReceiveMessage(Message message); | ||
Task StoreAsync(AgentState state, CancellationToken cancellationToken = default); | ||
Task<T> ReadAsync<T>(AgentId agentId, CancellationToken cancellationToken = default) where T : IMessage, new(); | ||
ValueTask PublishEventAsync(CloudEvent item, CancellationToken cancellationToken = default); | ||
ValueTask PublishEventAsync(string topic, IMessage evt, CancellationToken cancellationToken = default); |
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 used in other places (eg App.cs) why remove it?
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 IAgentBase
is already deleted by recent PR. So this change no longer apply.
Anyway, the idea of moving the overloaded PublishEventAsync
as an extension method is to simplify the interface definition
} | ||
var payload = item.ProtoData.Unpack(EventTypes.TypeRegistry); | ||
var convertedPayload = Convert.ChangeType(payload, EventTypes.Types[item.Type]); | ||
return this.HandleObject(convertedPayload); |
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 don't understand - with this all agents can handle all messages? I don't think that's what you want..... Maybe a quick call to discuss because I'm not understanding it.
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 HandleObject
will check if the agent has the right handler to be invoked. If there's no right handler a "Method not found on type {genericInterfaceType.FullName}"
will be raised.
Why are these changes needed?
Related issue number
fix #4354
Checks