-
Notifications
You must be signed in to change notification settings - Fork 7
add non-retryable errors, and shutdown helpers #33
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
Changes from 2 commits
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 |
|---|---|---|
|
|
@@ -233,6 +233,29 @@ class OrchestrationStateError(Exception): | |
| pass | ||
|
|
||
|
|
||
| class NonRetryableError(Exception): | ||
|
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. this is a new helper, that is present in Temporal but not us, where we can defined errors that are non-retryable so activities don't attempt to retry when raised |
||
| """Exception indicating the operation should not be retried. | ||
|
|
||
| If an activity or sub-orchestration raises this exception, retry logic will be | ||
| bypassed and the failure will be returned immediately to the orchestrator. | ||
| """ | ||
|
|
||
| pass | ||
|
|
||
|
|
||
| def is_error_non_retryable(error_type: str, policy: RetryPolicy) -> bool: | ||
| """Checks whether an error type is non-retryable.""" | ||
| is_non_retryable = False | ||
| if error_type == "NonRetryableError": | ||
| is_non_retryable = True | ||
| elif ( | ||
| policy.non_retryable_error_types is not None | ||
| and error_type in policy.non_retryable_error_types | ||
| ): | ||
| is_non_retryable = True | ||
| return is_non_retryable | ||
|
|
||
|
|
||
| class Task(ABC, Generic[T]): | ||
| """Abstract base class for asynchronous tasks in a durable orchestration.""" | ||
|
|
||
|
|
@@ -397,7 +420,7 @@ def compute_next_delay(self) -> Optional[timedelta]: | |
| next_delay_f = min( | ||
| next_delay_f, self._retry_policy.max_retry_interval.total_seconds() | ||
| ) | ||
| return timedelta(seconds=next_delay_f) | ||
| return timedelta(seconds=next_delay_f) | ||
|
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. this fixes a bug with retry, as the login in line 400 above
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. this is also kind of mentioned in one of the
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. added some info in README to cover the gotchas, but we might need to add to python-sdk |
||
|
|
||
| return None | ||
|
|
||
|
|
@@ -490,6 +513,7 @@ def __init__( | |
| backoff_coefficient: Optional[float] = 1.0, | ||
| max_retry_interval: Optional[timedelta] = None, | ||
| retry_timeout: Optional[timedelta] = None, | ||
| non_retryable_error_types: Optional[list[Union[str, type]]] = None, | ||
| ): | ||
| """Creates a new RetryPolicy instance. | ||
|
|
||
|
|
@@ -505,6 +529,11 @@ def __init__( | |
| The maximum retry interval to use for any retry attempt. | ||
| retry_timeout : Optional[timedelta] | ||
| The maximum amount of time to spend retrying the operation. | ||
| non_retryable_error_types : Optional[list[Union[str, type]]] | ||
| A list of exception type names or classes that should not be retried. | ||
| If a failure's error type matches any of these, the task fails immediately. | ||
| The built-in NonRetryableError is always treated as non-retryable regardless | ||
| of this setting. | ||
| """ | ||
| # validate inputs | ||
| if first_retry_interval < timedelta(seconds=0): | ||
|
|
@@ -523,6 +552,16 @@ def __init__( | |
| self._backoff_coefficient = backoff_coefficient | ||
| self._max_retry_interval = max_retry_interval | ||
| self._retry_timeout = retry_timeout | ||
| # Normalize non-retryable error type names to a set of strings | ||
| names: Optional[set[str]] = None | ||
| if non_retryable_error_types: | ||
acroca marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| names = set[str]() | ||
| for t in non_retryable_error_types: | ||
| if isinstance(t, str) and t: | ||
| names.add(t) | ||
| elif isinstance(t, type): | ||
| names.add(t.__name__) | ||
| self._non_retryable_error_types = names | ||
|
|
||
| @property | ||
| def first_retry_interval(self) -> timedelta: | ||
|
|
@@ -549,6 +588,15 @@ def retry_timeout(self) -> Optional[timedelta]: | |
| """The maximum amount of time to spend retrying the operation.""" | ||
| return self._retry_timeout | ||
|
|
||
| @property | ||
| def non_retryable_error_types(self) -> Optional[set[str]]: | ||
| """Set of error type names that should not be retried. | ||
|
|
||
| Comparison is performed against the errorType string provided by the | ||
| backend (typically the exception class name). | ||
| """ | ||
| return self._non_retryable_error_types | ||
|
|
||
|
|
||
| def get_name(fn: Callable) -> str: | ||
| """Returns the name of the provided function""" | ||
|
|
||
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.
add context manager option for clean closing