Conversation
andystaples
requested review from
berndverst,
Copilot and
halspang
and removed request for
Copilot
There was a problem hiding this comment.
Pull request overview
Adds an asyncio-friendly gRPC client to the Durable Task Python SDK (AsyncTaskHubGrpcClient) built on grpc.aio, and refactors shared request/interceptor/channel logic into internal helpers to keep sync and async clients aligned.
Changes:
- Introduces
AsyncTaskHubGrpcClientwith async equivalents of orchestration/entity operations. - Adds async gRPC channel creation (
get_async_grpc_channel) and async metadata interceptor support. - Adds async E2E coverage and updates test dependencies/configuration to run async pytest tests.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
durabletask/client.py |
Adds AsyncTaskHubGrpcClient and refactors shared logic usage for sync client. |
durabletask/internal/client_helpers.py |
New shared request/interceptor helper functions used by both clients. |
durabletask/internal/shared.py |
Adds get_async_grpc_channel and async interceptor typing. |
durabletask/internal/grpc_interceptor.py |
Adds DefaultAsyncClientInterceptorImpl and shared metadata application helper. |
tests/durabletask/test_orchestration_async_e2e.py |
New async E2E tests validating async client behavior with a sidecar. |
tests/durabletask/test_client.py |
Adds unit tests for async channel creation and async client construction. |
requirements.txt |
Adds pytest-asyncio for async testing. |
pyproject.toml |
Configures pytest-asyncio asyncio_mode = "auto". |
CHANGELOG.md |
Documents the new async client and related helpers. |
halspang
left a comment
halspang
left a comment
There was a problem hiding this comment.
Overall, I think this looks good! Just a few quick questions.
| >>> async with AsyncDurableTaskSchedulerClient( | ||
| ... host_address="my-dts-service.azure.com:443", | ||
| ... taskhub="my-task-hub", | ||
| ... token_credential=credential | ||
| ... ) as client: | ||
| ... instance_id = await client.schedule_new_orchestration("my_orchestrator") |
There was a problem hiding this comment.
So I asked Copilot about async python stuff and it is adamant that in order to do this, you need to implement __aenter__ and __aexit__. If you don't you'll get an attribute error. Is this real?
There was a problem hiding this comment.
Good catch, you learn something every day. Added, and updated the tests to use async with to avoid leaking the channel.
| if token_credential is not None: | ||
| self._token_credential = token_credential | ||
| self._token_manager = AccessTokenManager(token_credential=self._token_credential) | ||
| access_token = self._token_manager.get_access_token() |
There was a problem hiding this comment.
Does this make a call out and/or does it need to be async for this interceptor?
There was a problem hiding this comment.
Yes, AccessTokenManager was implemented using the sync azure methods, the azure SDK does export async versions, so I added AsyncAccessTokenManager using AsyncTokenCredential and made these calls run asynchronously.
Adds an async version of TaskHubGrpcClient (AsyncTaskHubGrpcClient) that offers async-aware client methods using grpc.aio's async gRPC calls. Refactors code shared by both clients to /internal/client_helpers.py.
Compare to earlier open-source contrib #65 / dapr#17
Resolves #64