fix: apply DAPR_API_TIMEOUT_SECONDS to workflow gRPC connections#954
fix: apply DAPR_API_TIMEOUT_SECONDS to workflow gRPC connections#954
Conversation
The workflow extension's gRPC connections (DaprWorkflowClient, async DaprWorkflowClient, and WorkflowRuntime) were not respecting the DAPR_API_TIMEOUT_SECONDS environment variable, unlike the core SDK's DaprGrpcClient which applies it via a timeout interceptor. Pass DaprClientTimeoutInterceptor (sync) and DaprClientTimeoutInterceptorAsync (async) to the durabletask TaskHubGrpcClient, AsyncTaskHubGrpcClient, and TaskHubGrpcWorker so that workflow gRPC calls get the configured default timeout. Signed-off-by: Fabian Martinez <fabian@diagrid.io> Signed-off-by: Fabian Martinez <46371672+famarting@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR ensures workflow-related gRPC connections (workflow client, async workflow client, and workflow runtime worker) respect the global DAPR_API_TIMEOUT_SECONDS default by adding the SDK’s timeout interceptor to the underlying durabletask gRPC channels.
Changes:
- Add
DaprClientTimeoutInterceptorto the durabletask worker channel inWorkflowRuntime, while preserving user-provided interceptors. - Add
DaprClientTimeoutInterceptorto the syncDaprWorkflowClientdurabletask channel. - Add
DaprClientTimeoutInterceptorAsyncto the asyncDaprWorkflowClientdurabletask channel.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ext/dapr-ext-workflow/dapr/ext/workflow/workflow_runtime.py | Prepends timeout interceptor and merges it with any user-provided interceptors for the worker channel. |
| ext/dapr-ext-workflow/dapr/ext/workflow/dapr_workflow_client.py | Adds timeout interceptor to the durabletask sync workflow client channel. |
| ext/dapr-ext-workflow/dapr/ext/workflow/aio/dapr_workflow_client.py | Adds timeout interceptor to the durabletask async workflow client channel. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.__obj = client.TaskHubGrpcClient( | ||
| host_address=uri.endpoint, | ||
| metadata=metadata, | ||
| secure_channel=uri.tls, | ||
| log_handler=options.log_handler, | ||
| log_formatter=options.log_formatter, | ||
| interceptors=[DaprClientTimeoutInterceptor()], | ||
| ) |
| self.__obj = aioclient.AsyncTaskHubGrpcClient( | ||
| host_address=uri.endpoint, | ||
| metadata=metadata, | ||
| secure_channel=uri.tls, | ||
| log_handler=options.log_handler, | ||
| log_formatter=options.log_formatter, | ||
| interceptors=[DaprClientTimeoutInterceptorAsync()], | ||
| ) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## main #954 +/- ## ========================================== + Coverage 86.63% 89.16% +2.52% ========================================== Files 84 104 +20 Lines 4473 7417 +2944 ========================================== + Hits 3875 6613 +2738 - Misses 598 804 +206 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Fabian Martinez <46371672+famarting@users.noreply.github.com>
sicoyle left a comment
sicoyle left a comment There was a problem hiding this comment.
I see your changes update only the aio client. Can you pls update the one here too python-sdk/ext/dapr-ext-workflow/dapr/ext/workflow/dapr_workflow_client.py and also pls add tests to make the build happy for code cov 🙏
@sicoyle I'm confused, the file |
@famarting I only know bc I have missed the other client myself in past PRs hehe. Your PR file changes are for Context: There are two separate clients one that is synchronous and uses |
this PR is already updating |
Summary
DaprWorkflowClient, asyncDaprWorkflowClient,WorkflowRuntime) were not respectingDAPR_API_TIMEOUT_SECONDSDaprClientTimeoutInterceptor/DaprClientTimeoutInterceptorAsyncto the durabletask client and worker gRPC channelsWorkflowRuntimeare preserved alongside the timeout interceptorTest plan
DAPR_API_TIMEOUT_SECONDSset to a custom value that workflow gRPC calls respect ittimeout_in_secondsonwait_for_workflow_completion) still take precedence