Declutter __init__ by using strategy selection

[harlowja]

Instead of having there be a complicated __init__
function that has to make a bunch of complicated
decisions this change moves the logic for making
those decisions into a set of helper functions that
now select the various wait/stop/reject strategies
to use when retrying.

This will hopefully make it easier to add new
strategies as well as move forward to a path where
users can provide there own strategies (if they
so want to).

[bodenr]

@harlowja @jd @rholder

My initial $0.01:
While I do think this is a good refactor atop the current base of code; IMHO if we are going this far we should consider a major revision of this project whereupon we change the public interface.

In particular, I'm not a huge fan of the current init scheme lumping all args together as I find it confusing on the behavior if you specify values for different wait schemes (for example).

I'd prefer a more modular interface where retrying provides (pluggable/composable) building blocks for retry behavior. For example basic "abstract interfaces" for the major pluggable behavior (sleep time, stop, etc.), chaining, etc.

I was imagining something like this usage for example:

# simple fixed waited and stop after attempts
@retry(wait=retrying.FixedWait(1000), stop=retrying.StopOnMaxAttempt(7))
def basic_retry():
    useful_stuff()


# chained behavior
wait_chain = retrying.CallChain(retrying.FixedWait(1000),
                                my_custom_fn)
stop_chain = retrying.CallChain(retrying.StopOnMaxDelay(11.21),
                                my_custom_stop_fn)
@retry(wait=wait_chain, stop=stop_chain)
def chained_retry():
    more_useful_stuff()


# custom plugins
class MyWait(retrying.WaitBase):

    def sleep(self, attempts, prev):
        return prev * attempts


@retry(wait=MyWait(), stop=retrying.StopOnMaxAttempt(7))
def custom_retry():
    useful_things()

Are you guys open to a major change (major revision), or should we just continue to enhance the existing interface?

[harlowja]

Yes, I'd like the building blocks u state; which I think this strategy 'refactor' can help us get to. Should it just be a major revision, probably. Should we go farther and expose those strategies for public use/changing/chaining... in that revision, maybe?

[bodenr]

Fair enough. Let me find some time tomorrow (hopefully) to review your PR.

[bodenr]

@harlowja
After sleeping on it, I'm still thinking it would make the most sense for us to leave the current impl as-is and focus on the next major revision (the interface/refactor change we discussed above). While I certainly agree the refactor in this PR is a welcomed change to the current structure, I don't see what it buys us if we are planning to change the interface in a major revision.

I'd be happy to take a 1st shot at approximating a new revision if you guys are willing to consider it as a starting/talking point?

If you guys all think the refactor in this PR should move forward; I'm certainly on-board and willing to review, but again I'd rather see us focus on a major revision with interface changes.

Just my $0.01

[harlowja]

@bodenr I don't disagree. I'd just rather have the owner/creator of retrying chime-in before we do such a retrying 2.0 (or whatever u want to call it).

[bodenr]

@rholder what are your thought in regards to the discussion above (e.g. updated/new interface for a "major revision" of retrying)?

[bodenr]

Hi @rholder

Any chance you can try to weigh-in on this thread?

Based on discussions we've had (see comments herein as well as in [1][2]), enhancing retrying's modularity/pluggability is paramount to supporting new functionality in a clean manner.

I'm more than happy to drive this work; just looking for your blessing :)

Thanks

[1] #50
[2] #51