temporalio / sdk-python

Temporal Python SDK
MIT License
473 stars 77 forks source link

Add WorkflowUpdateRPCTimeoutOrCancelledError #548

Closed cretz closed 5 months ago

cretz commented 5 months ago

What was changed

See https://github.com/temporalio/features/issues/483. I will update current features repo Python CI branch just before merging this.

Checklist

  1. Closes #529
drewhoskins-temporal commented 5 months ago

All of these are fine. If you like ...ClientTimeoutOrCancelled, it seems flexible enough. (DeadlineExceeded doesn't help me understand that it includes cancel, personally. I'm good with the explicitness of what you did.)

On Tue, Jun 11, 2024 at 05:38 Chad Retz @.***> wrote:

@.**** commented on this pull request.

In temporalio/client.py https://github.com/temporalio/sdk-python/pull/548#discussion_r1634810986 :

@@ -4456,6 +4478,24 @@ def cause(self) -> BaseException: return self.cause

+class RPCTimeoutOrCancelledError(temporalio.exceptions.TemporalError):

  • """Error that occurs on some client calls that timeout or get cancelled."""
  • pass
  • +class WorkflowUpdateRPCTimeoutOrCancelledError(RPCTimeoutOrCancelledError):

we've already crossed that bridge

Not necessarily. This is a brand new thing to wrap these kinds of gRPC errors, so we can change. We can have ClientTimeoutError and WorkflowUpdateClientTimeoutError, but need to confirm that that also includes Cancelled status codes and asyncio.Cancelled cancellation or if we just want it as DeadlineExceeded (I am not sure that "timeout" is a good name for something that includes cancellation).

— Reply to this email directly, view it on GitHub https://github.com/temporalio/sdk-python/pull/548#discussion_r1634810986, or unsubscribe https://github.com/notifications/unsubscribe-auth/BHV3GXP4ITVSHX2DSWTTKD3ZG3VVTAVCNFSM6AAAAABJDDCPC6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMJQGMZTOOBVGE . You are receiving this because you commented.Message ID: @.***>