Skip to content

Commit 6e93145

Browse files
barmosheclaude
andcommitted
Address review feedback on reqrespupdate
- Bound the test retry helper with a max attempt count and a short sleep, so a workflow that stops continuing as new fails the test instead of retrying forever and hanging CI. - Skip both tests on the time-skipping server, matching the other update tests in the repo (temporalio/sdk-java#1903). - Reject requests_before_continue_as_new < 1 in the workflow constructor. Zero made the run continue as new immediately and forever while rejecting every request. - Move TASK_QUEUE and WORKFLOW_ID into __init__.py, as sleep_for_days and message_passing/waiting_for_handlers do, instead of the worker hardcoding the task queue name. - Use the existing handle in the continue-as-new test rather than re-fetching it; start_workflow does not pin the handle to a run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 4245b7c commit 6e93145

6 files changed

Lines changed: 39 additions & 12 deletions

File tree

reqrespupdate/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
TASK_QUEUE = "reqrespupdate-task-queue"
2+
WORKFLOW_ID = "reqrespupdate-workflow-id"

reqrespupdate/requester.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from temporalio.envconfig import ClientConfig
55
from temporalio.exceptions import ApplicationError
66

7-
from reqrespupdate.starter import WORKFLOW_ID
7+
from reqrespupdate import WORKFLOW_ID
88
from reqrespupdate.workflow import BACKOFF_ERROR_TYPE, Request, UppercaseWorkflow
99

1010

reqrespupdate/starter.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,9 @@
33
from temporalio.client import Client
44
from temporalio.envconfig import ClientConfig
55

6+
from reqrespupdate import TASK_QUEUE, WORKFLOW_ID
67
from reqrespupdate.workflow import UppercaseWorkflow, UppercaseWorkflowInput
78

8-
WORKFLOW_ID = "reqrespupdate-workflow-id"
9-
TASK_QUEUE = "reqrespupdate-task-queue"
10-
119

1210
async def main():
1311
config = ClientConfig.load_client_connect_config()

reqrespupdate/worker.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from temporalio.envconfig import ClientConfig
66
from temporalio.worker import Worker
77

8+
from reqrespupdate import TASK_QUEUE
89
from reqrespupdate.activities import uppercase
910
from reqrespupdate.workflow import UppercaseWorkflow
1011

@@ -18,7 +19,7 @@ async def main():
1819

1920
async with Worker(
2021
client,
21-
task_queue="reqrespupdate-task-queue",
22+
task_queue=TASK_QUEUE,
2223
workflows=[UppercaseWorkflow],
2324
activities=[uppercase],
2425
):

reqrespupdate/workflow.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ class UppercaseWorkflow:
5353

5454
@workflow.init
5555
def __init__(self, input: UppercaseWorkflowInput) -> None:
56+
# A run has to accept at least one request, otherwise it continues as
57+
# new the moment it starts and the chain never does any work.
58+
if input.requests_before_continue_as_new < 1:
59+
raise ApplicationError(
60+
"requests_before_continue_as_new must be at least 1",
61+
non_retryable=True,
62+
)
5663
self.input = input
5764
self.request_count = 0
5865

tests/reqrespupdate/workflow_test.py

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1+
import asyncio
12
import uuid
23

4+
import pytest
35
from temporalio.client import Client, WorkflowUpdateFailedError
46
from temporalio.exceptions import ApplicationError
7+
from temporalio.testing import WorkflowEnvironment
58
from temporalio.worker import Worker
69

710
from reqrespupdate.activities import uppercase
@@ -13,12 +16,14 @@
1316
)
1417

1518

16-
async def request_uppercase(handle, text: str) -> str:
19+
async def request_uppercase(handle, text: str, max_attempts: int = 10) -> str:
1720
"""Request an uppercasing, retrying if the workflow is continuing as new.
1821
19-
This is the same backoff the requester in this sample performs.
22+
This is the same backoff the requester in this sample performs, bounded so
23+
that a workflow which never continues as new fails the test instead of
24+
retrying forever.
2025
"""
21-
while True:
26+
for _ in range(max_attempts):
2227
try:
2328
response = await handle.execute_update(
2429
UppercaseWorkflow.uppercase, Request(input=text)
@@ -29,11 +34,19 @@ async def request_uppercase(handle, text: str) -> str:
2934
isinstance(err.cause, ApplicationError)
3035
and err.cause.type == BACKOFF_ERROR_TYPE
3136
):
37+
await asyncio.sleep(0.1)
3238
continue
3339
raise
40+
raise AssertionError(
41+
f"Request for {text} still rejected after {max_attempts} attempts"
42+
)
3443

3544

36-
async def test_uppercase(client: Client):
45+
async def test_uppercase(client: Client, env: WorkflowEnvironment):
46+
if env.supports_time_skipping:
47+
pytest.skip(
48+
"Java test server: https://github.com/temporalio/sdk-java/issues/1903"
49+
)
3750
task_queue = f"tq-{uuid.uuid4()}"
3851
async with Worker(
3952
client,
@@ -54,7 +67,13 @@ async def test_uppercase(client: Client):
5467
await handle.terminate()
5568

5669

57-
async def test_continues_as_new_without_losing_requests(client: Client):
70+
async def test_continues_as_new_without_losing_requests(
71+
client: Client, env: WorkflowEnvironment
72+
):
73+
if env.supports_time_skipping:
74+
pytest.skip(
75+
"Java test server: https://github.com/temporalio/sdk-java/issues/1903"
76+
)
5877
task_queue = f"tq-{uuid.uuid4()}"
5978
async with Worker(
6079
client,
@@ -78,7 +97,7 @@ async def test_continues_as_new_without_losing_requests(client: Client):
7897
for i in range(6):
7998
assert await request_uppercase(handle, f"foo{i}") == f"FOO{i}"
8099

81-
description = await client.get_workflow_handle(handle.id).describe()
100+
description = await handle.describe()
82101
assert description.run_id != handle.first_execution_run_id
83102
finally:
84-
await client.get_workflow_handle(handle.id).terminate()
103+
await handle.terminate()

0 commit comments

Comments
 (0)