-
Notifications
You must be signed in to change notification settings - Fork 913
.NET: Use GrpcEntityRunner instead of TaskEntityDispatcher #2759
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
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.
Pull request overview
This PR updates the durable agent framework to use GrpcEntityRunner directly instead of TaskEntityDispatcher, addressing a compatibility issue with recent changes to the durable worker extension where TaskEntityDispatcher is no longer initialized by the time the agent executor runs.
Key Changes:
- Replaced
TaskEntityDispatcherparameter withstringparameter in entity invocation methods - Updated implementation to call
GrpcEntityRunner.LoadAndRunAsyncdirectly - Reordered parameters for consistency with other methods in the codebase
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| dotnet/src/Microsoft.Agents.AI.Hosting.AzureFunctions/BuiltInFunctions.cs | Changed InvokeAgentAsync method signature to accept string instead of TaskEntityDispatcher, removed async keyword, and updated implementation to use GrpcEntityRunner.LoadAndRunAsync directly |
| dotnet/src/Microsoft.Agents.AI.Hosting.AzureFunctions/BuiltInFunctionExecutor.cs | Updated variable type from TaskEntityDispatcher to string, changed type matching in switch statement, and updated call site to match new parameter order |
| [EntityTrigger] TaskEntityDispatcher dispatcher, | ||
| public static Task<string> InvokeAgentAsync( | ||
| [DurableClient] DurableTaskClient client, | ||
| string dispatcher, |
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.
instead of dispatcher, should we be considering naming this encodedEntityRequest? (That is the parameter name of LoadAndRunAsync method to which we are passing this value to.)
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.
For binding, the name needed to match the generated function metadata. I could update that though if you want.
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.
We have the freedom to update the metadata value. I will leave it up to you to use the readable name for this one since you are more familiar with the durable concepts.
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.
Renamed it. Can you do one more round of testing? I still can't get my local environment to work due to lack of permissions.
kshyju
left a comment
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.
Can we also update the changelogs.md?
Motivation and Context
Resolves #2327
Description
Durable agent framework pipeline was taking a dependency on the precise way the durable worker extension bootstrapped an entity invocation. With a recent change to the durable extension this no longer worked as that bootstrapping was deferred. This PR reduces that dependency by removing usage of
TaskEntityDispatcher(which is no longer setup by the time this executor runs) and instead usesGrpcEntityRunner.TaskEntityDispatcheris just a wrapper aroundGrpcEntityRunneranyways.Another reason reducing these bootstrapping dependencies is important is that it is inherently fragile. It requires the durable functions worker middleware run first, but there is nothing guaranteeing the middleware order between the extensions today. It works right now because durable tends to be configured from source gen auto-start hooks, but that is not set in stone that it will always work that way. For long term stability it will be better to decouple the durable agent's invocations from these implementation details of the durable function worker extension.
Contribution Checklist