-
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 AgentChat #4561
base: main
Are you sure you want to change the base?
.NET AgentChat #4561
Conversation
GroupChat[Manager]Base [ ] protobuf the messages, otherwise cannot call into Publish() [ ] flesh out Reset() [ ] Other group chat strategies
@@ -171,6 +171,8 @@ public List<string> Subscribe(string topic) | |||
} | |||
} | |||
}; | |||
|
|||
// TODO: Should this be async? |
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.
was purposefully not sync - but should have a try/catch/error handling or retry - we want to make sure that subscriptions complete because if we think they are there but aren't there yet we may miss messages we thought we were getting...probably better ways to handle this.
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.
Right, that makes sense.
I would still consider having an Async method under the hood that could be used in an async context (and also provide the Sync method which does this same kind of direct wait on SubscribeAsync
) is that it will avoid locking up a thread that the async framework thinks belongs to it - since in an async context await (awaitable)
will block the logical flow until the result, similar to .Wait()
in a synchronous context.
* Make the AgentChat message types consistent with Python
Abstractions
Agents
Teams
Other tasks