feature: retry decorator improvements #2390
Conversation
…tion monitor sidecar changes
…itting only_on attribute
| only_on : List[str], default None | ||
| List of failure events to retry on. Not specifying values will retry on all known events. | ||
| Accepted values are | ||
| 'step', 'instance-preemption' |
There was a problem hiding this comment.
this can be all, instance-preemption. all can be the default value and is the same as the current behavior.
There was a problem hiding this comment.
but do we even need the all as a value, as that behaviour should be covered by omitting the attribute completely? is it also not a plausible use-case to retry only on user-code exceptions?
| from metaflow import current | ||
|
|
||
|
|
||
| class RetryEvents(Enum): |
There was a problem hiding this comment.
its mainly for convenience when building the retry conditions on the executor implemention side, where we need to go through the supported options. Having a literal value to use instead of retyping strings reduces issues with possible typos or renames down the line.
for example this:
RetryEvents.PREEMPT.value in retry_conditions
metaflow/plugins/retry_decorator.py
Outdated
| PREEMPT = "instance-preemption" | ||
|
|
||
|
|
||
| PLATFORM_EVICTED_EXITCODE = 234 |
There was a problem hiding this comment.
is this needed here - since technically we might have many different exit codes depending on the error being caught. also depending on the error backend, the exit codes can be different for the same conceptual failure mode.
There was a problem hiding this comment.
move to kubernetes / batch respectively. Kept the exit code the same for now as there shouldn't be any conflicts with the current value, but this can be changed as needed.
introduces
only_on=to the retry decorator in order to conditionally retry on different types of failures