Use retrying client for worker deployment RPCs in tests - #1829
Draft
DABH wants to merge 2 commits into
Draft
Conversation
The frontend recycles client connections after frontend.keepAliveMaxConnectionAge (5 minutes, jittered) by sending GOAWAY. While worker long polls keep the retiring connection open, the first request queued on it fails inside hyper with "operation was canceled", which tonic surfaces as gRPC CANCELLED. The sdk-core retry client recognizes this transport-sourced CANCELLED and retries it, but the deployment test helpers called the raw service stubs with the default retry=False, so once per connection lifetime a SetWorkerDeploymentCurrentVersion or SetWorkerDeploymentRampingVersion call could fail with RPCError: (1, 'operation was canceled', b''). Route the helpers through the retrying client, as the high-level Client does for its own RPCs.
There was a problem hiding this comment.
🟢 Approval recommended
The focused changes consistently enable the intended retry path without altering test semantics.
Pull request overview
Enables retry handling for worker deployment test helpers to prevent transient connection-recycling failures.
Changes:
- Opts deployment describe and mutation RPCs into SDK Core retries.
- Covers visibility and routing-propagation polling helpers.
File summaries
| File | Description |
|---|---|
tests/worker/test_worker.py |
Enables retries for four direct worker deployment RPC calls. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
The worker deployment helpers in
tests/worker/test_worker.pycall the raw service stubs withretry=True.Why
The frontend recycles client connections every 5 minutes with an HTTP/2 GOAWAY. A worker's long polls keep the old connection open, so hyper only notices when the next request is queued and drops it as
CANCELLED ('operation was canceled')before it reaches the server. sdk-core retries transport-level cancels, but only through the retry client, which these helpers bypassed. CI sessions outlive the 5-minute window, so the first raw non-retried RPC after a recycle fails. Temporal Cloud recycles the same way.Testing
Dev server with
frontend.keepAliveMaxConnectionAge="5s", four deployment tests x10: 9/40 failed before, 40/40 after. Default config,-k deploymentx8 under xdist: 48/48. Lint clean.