Feature - Raise on Giveup #129
Conversation
backoff/_async.py
Outdated
| @@ -131,7 +131,7 @@ def retry_exception(target, wait_gen, exception, | |||
| async def retry(*args, **kwargs): | |||
|
|
|||
| # update variables from outer function args | |||
| nonlocal max_tries, max_time | |||
| nonlocal max_tries, max_time, raise_on_giveup | |||
There was a problem hiding this comment.
I don't think we need to add raise_on_giveup to the nonlocal list here, because unlike max_tries and max_time, we are not reassigning the variable with the output of _maybe_call()
backoff/_async.py
Outdated
| @@ -158,8 +158,10 @@ async def retry(*args, **kwargs): | |||
| elapsed >= max_time) | |||
|
|
|||
| if giveup_result or max_tries_exceeded or max_time_exceeded: | |||
| await _call_handlers(on_giveup, **details) | |||
| raise | |||
| handler_result = await _call_handlers(on_giveup, **details) | |||
There was a problem hiding this comment.
I don't think this works as is, because on_giveup is actually a list of handlers here. The user facing API accepts either a list or a single handler for on_giveup, but in this internal code path it has already been wrapped in a list if it was a single handler function. The question then is handler_result is the return value of which handler. See the implementation of _call_handlers() to see what I mean. I liked the thought of having the result value be that of the on_giveup handler, but I doubt think that works in practice. But let me know if you have any bright ideas.
There was a problem hiding this comment.
Right, of course! I guess just a bare return or return None (and not promising to return any values from on_giveup handlers) makes the most sense maybe?
There was a problem hiding this comment.
Updated to return explicit None
#118
Implement feature to optionally disable raising of exceptions on giveup.