Some comments first, then addressing issues:
I thought it might help if I walked you through my thought process of
making this change.
* the purpose of the retry package is to provide a way to retry actions
that may fail until they succeed, or some predetermined limit is hit
* the way that failure is idiomatically handled is with an error
* I wanted the interface to be very simple to use, and hard to get wrong
* it should work inline in a simple for loop
* the library should make the simple case very simple
var result Foo
var err error
for loop := retry.Loop(spec); loop.Next(err); {
result, err = SomeFunc(blah)
}
if err != nil {
// what ever
}
// continue using result
What about fatal errors?
var result Foo
var err error
for loop := retry.Loop(spec); loop.Next(err); {
result, err = SomeFunc(blah)
if isFatalErr(err) {
break
}
}
if err != nil {
// what ever
}
// continue using result
On 21/10/16 06:33, roger peppe wrote:
On 20 October 2016 at 16:30, Katherine Cox-Buday
<katherine.cox-bu...@canonical.com> wrote:
John Meinel <j...@arbash-meinel.com> writes:
As commented on the pull request, passing 'err' into Next() initially feels
weird, but
it allows a big improvement to whether the loop exited because we ran out of
retries, or it exited because the request succeeded. (loop.Error()
tells us either way.)
I don't understand why the retry logic has the concern of why the loop exits
(i.e. Next(error)).
The rationale behind that is to make the the library handle the 80% case
easily. The main reason of passing the error into Next is so you don't
have to do the check and explicit break within the loop.
The motivation of moving towards loops was so that the concern of what's being
retried is brought back to be inline with the surrounding code. Having the
retry mechanism inspecting errors doesn't fall in line with that goal.
In my mind, any retry solutions only needs to cover two things:
1. Provide a primitive that will delay a for iteration in a controlled way.
2. Be preemptable.
Everything else is the logic of the thing you're retrying, including why it
eventually succeeded/failed.
Here is were we disagree slightly. I also think that very high on the
list are:
* easy to get right
* hard to get wrong
That's well put, and fits well with my thoughts too, thanks.
See also Pawel Stolowski's comment on
https://github.com/juju/retry/pull/5. He's from the Snappy
team who have also been trying to move towards using a standard retry mechanism.
There are many possible reasons for wanting to retry - error values
are just one possibility.
Sure, but this could be trivially handled by assigning something to err
(consider if we made a public error in the retry package for this):
var result Foo
var err error
for loop := retry.Loop(spec); loop.Next(err); {
result, err = SomeFunc(blah)
if err == nil && resultNotGoodEnough(result) {
err = retry.ErrTryAgain
}
}
if err != nil {
// what ever
}
// continue using result
Tim
--
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/juju-dev
