-
Notifications
You must be signed in to change notification settings - Fork 155
Add support for more timeouts to Nexus operations #1276
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?
Changes from all commits
d337838
2100a07
8d0939d
c1e512e
c7b0ff0
a780707
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ | |
| ApplicationError, | ||
| NexusOperationError, | ||
| TimeoutError, | ||
| TimeoutType, | ||
| ) | ||
| from temporalio.testing import WorkflowEnvironment | ||
| from temporalio.worker import Worker | ||
|
|
@@ -323,6 +324,149 @@ async def test_error_raised_by_timeout_of_nexus_start_operation( | |
| assert capturer.find_log("unexpected cancellation reason") is None | ||
|
|
||
|
|
||
| # Schedule to start timeout test | ||
| @service_handler | ||
| class ScheduleToStartTimeoutTestService: | ||
| @sync_operation | ||
| async def expect_schedule_to_start_timeout( | ||
| self, ctx: StartOperationContext, _input: None | ||
| ) -> None: | ||
| try: | ||
| await asyncio.wait_for(ctx.task_cancellation.wait_until_cancelled(), 1) | ||
| except asyncio.TimeoutError: | ||
| raise ApplicationError("expected cancel", non_retryable=True) | ||
|
|
||
|
|
||
| @workflow.defn | ||
| class ScheduleToStartTimeoutTestCallerWorkflow: | ||
| @workflow.init | ||
| def __init__(self): | ||
| self.nexus_client = workflow.create_nexus_client( | ||
| service=ScheduleToStartTimeoutTestService, | ||
| endpoint=make_nexus_endpoint_name(workflow.info().task_queue), | ||
| ) | ||
|
|
||
| @workflow.run | ||
| async def run(self) -> None: | ||
| await self.nexus_client.execute_operation( | ||
| ScheduleToStartTimeoutTestService.expect_schedule_to_start_timeout, | ||
| None, | ||
| output_type=None, | ||
| schedule_to_start_timeout=timedelta(seconds=0.1), | ||
| ) | ||
|
|
||
|
|
||
| async def test_error_raised_by_schedule_to_start_timeout_of_nexus_operation( | ||
| client: Client, env: WorkflowEnvironment | ||
| ): | ||
| if env.supports_time_skipping: | ||
| pytest.skip("Nexus tests don't work with time-skipping server") | ||
|
|
||
| task_queue = str(uuid.uuid4()) | ||
| async with Worker( | ||
| client, | ||
| nexus_service_handlers=[ScheduleToStartTimeoutTestService()], | ||
| workflows=[ScheduleToStartTimeoutTestCallerWorkflow], | ||
| task_queue=task_queue, | ||
| nexus_task_executor=concurrent.futures.ThreadPoolExecutor(), | ||
| ): | ||
| await env.create_nexus_endpoint( | ||
| make_nexus_endpoint_name(task_queue), task_queue | ||
| ) | ||
| try: | ||
| await client.execute_workflow( | ||
| ScheduleToStartTimeoutTestCallerWorkflow.run, | ||
| id=str(uuid.uuid4()), | ||
| task_queue=task_queue, | ||
| ) | ||
| except Exception as err: | ||
| assert isinstance(err, WorkflowFailureError) | ||
| assert isinstance(err.__cause__, NexusOperationError) | ||
| assert isinstance(err.__cause__.__cause__, TimeoutError) | ||
| timeout_err = err.__cause__.__cause__ | ||
| assert timeout_err.type == TimeoutType.SCHEDULE_TO_START | ||
| else: | ||
| pytest.fail( | ||
| "Expected exception due to schedule to start timeout of nexus operation" | ||
| ) | ||
|
|
||
|
|
||
| # Start to close timeout test | ||
|
|
||
|
|
||
| class OperationThatExpectsStartToCloseTimeoutAsync(OperationHandler[None, None]): | ||
| async def start( | ||
| self, ctx: StartOperationContext, input: None | ||
| ) -> StartOperationResultAsync: | ||
| return StartOperationResultAsync("fake-token") | ||
|
|
||
| async def cancel(self, ctx: CancelOperationContext, token: str) -> None: | ||
| pass | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| @service_handler | ||
| class StartToCloseTimeoutTestService: | ||
| @operation_handler | ||
| def expect_start_to_close_timeout(self) -> OperationHandler[None, None]: | ||
| return OperationThatExpectsStartToCloseTimeoutAsync() | ||
|
|
||
|
|
||
| @workflow.defn | ||
| class StartToCloseTimeoutTestCallerWorkflow: | ||
| @workflow.init | ||
| def __init__( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any particular reason to create the nexus client as a member? Not that it matters too much in the test, but if we're viewing all code as AI consumed, is this a good pattern to suggest?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's actually what all the tests in this file do so i just copied them for consistency. |
||
| self, | ||
| ): | ||
| self.nexus_client = workflow.create_nexus_client( | ||
| service=StartToCloseTimeoutTestService, | ||
| endpoint=make_nexus_endpoint_name(workflow.info().task_queue), | ||
| ) | ||
|
|
||
| @workflow.run | ||
| async def run(self) -> None: | ||
| op_handle = await self.nexus_client.start_operation( | ||
| StartToCloseTimeoutTestService.expect_start_to_close_timeout, | ||
| None, | ||
| start_to_close_timeout=timedelta(seconds=0.1), | ||
| ) | ||
| await op_handle | ||
|
|
||
|
|
||
| async def test_error_raised_by_start_to_close_timeout_of_nexus_operation( | ||
| client: Client, env: WorkflowEnvironment | ||
| ): | ||
| if env.supports_time_skipping: | ||
| pytest.skip("Nexus tests don't work with time-skipping server") | ||
|
|
||
| task_queue = str(uuid.uuid4()) | ||
| async with Worker( | ||
| client, | ||
| nexus_service_handlers=[StartToCloseTimeoutTestService()], | ||
| workflows=[StartToCloseTimeoutTestCallerWorkflow], | ||
| task_queue=task_queue, | ||
| nexus_task_executor=concurrent.futures.ThreadPoolExecutor(), | ||
| ): | ||
| await env.create_nexus_endpoint( | ||
| make_nexus_endpoint_name(task_queue), task_queue | ||
| ) | ||
| try: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer |
||
| await client.execute_workflow( | ||
| StartToCloseTimeoutTestCallerWorkflow.run, | ||
| id=str(uuid.uuid4()), | ||
| task_queue=task_queue, | ||
| ) | ||
| except Exception as err: | ||
| assert isinstance(err, WorkflowFailureError) | ||
| assert isinstance(err.__cause__, NexusOperationError) | ||
| timeout_err = err.__cause__.__cause__ | ||
| assert isinstance(timeout_err, TimeoutError) | ||
| assert timeout_err.type == TimeoutType.START_TO_CLOSE | ||
| else: | ||
| pytest.fail( | ||
| "Expected exception due to start to close timeout of nexus operation" | ||
| ) | ||
|
|
||
|
|
||
| # Cancellation timeout test | ||
|
|
||
|
|
||
|
|
||
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.
StartNexusOperationInputis part of the public interceptor API, but these newly added fields are required constructor args, which is a backwards-incompatible break for existing interceptors that instantiate this dataclass (or pass positional args) using the prior signature. After this change, older code will either raiseTypeErrorfor missing args or mis-bind positional values, so adding defaults (e.g.,None) is needed to preserve compatibility while introducing the new timeouts.Useful? React with 👍 / 👎.