search

rholder / retrying

Retrying is an Apache 2.0 licensed general-purpose retrying library, written in Python, to simplify the task of adding retry behavior to just about anything.
Apache License 2.0
1.91k stars 157 forks source link

Support for retry event hook #50

Open bodenr opened 8 years ago

bodenr commented 8 years ago

It's very common for consumers to perform some action on each attempt iteration; for example logging a message that the operation failed and a retry will be performed after some time. While this can be done today using a custom wait_func, it's inconvenient to use a partial to wrap an existing retrying sleep function just to get this behavior.

This patch adds support for a new kwarg called wait_event_func that when passed should reference a function to be called before sleeping for another attempt iteration. This function looks similar to retrying wait_funcs except its first arg is the time to sleep as returned by the current 'wait' function.

A handful of unit tests are also included.

bodenr commented 8 years ago

@jd @harlowja @rholder I realize this PR has conflicts now, but I'd like to understand your initial impressions of this code to determine if it's worth my time continuing here. Thanks

jd commented 8 years ago

LGTM, feel free to rebase!

bodenr commented 8 years ago

travis didn't rebuild; let me try closing + reopening this

bodenr commented 8 years ago

@jd looks like this one is clean now. My apologies for all the noise here; I took a bumpy path on a git adventure.

harlowja commented 8 years ago

Seems pretty useful to me :+1:

bodenr commented 8 years ago

@jd @rholder what do you guys think; is this merge-worthy?

jd commented 8 years ago

Looks redundant with wait_func, no?

bodenr commented 8 years ago

@jd perhaps.

As indicated in the "commit message" its very common to want a "callback" per retry (for something like logging a message). While this could be done with a custom wait_func it inconvenient to decorate and provide a custom wait_func just for this purpose. That said the intent here is to simplify that consumption pattern.

jd commented 8 years ago

I think we don't want to clutter more the init func. What about providing that functionality as part of a custom wait_func that would be provided by retrying itself?

Possibly a composable class.

harlowja commented 8 years ago

I agree with jd (also why I did https://github.com/rholder/retrying/pull/55 to help move toward a model that hopefully can be more composeable and user-friendly); so perhaps we should get #55 in first?

bodenr commented 8 years ago

@jd @harlowja I just pushed a commit here with some changes I was playing with that introduce function chaining using the CallChain class I whipped up (no UTs; more of a talking point).

Using this approach it's easy to just pass my "hook function" as the wait_func to achieve what I need. It also address the "chaining" TODO in the code if I understand properly.

I haven't see #55 yet, so perhaps we can weight pros/cons of it vs. the CallChain I just committed to this PR.