Re: juju/retry take 2 - looping

2016-10-26 Thread roger peppe
On 26 October 2016 at 04:46, John Meinel wrote: >> I think this is a hint that this is the wrong approach. The edge-cases >> begin showing the cracks in the abstraction and end up making the code more >> complex. Consider your example instead of: >> >> var finalResult Foo

Re: juju/retry take 2 - looping

2016-10-25 Thread John Meinel
... > > I think this is a hint that this is the wrong approach. The edge-cases > begin showing the cracks in the abstraction and end up making the code more > complex. Consider your example instead of: > > var finalResult Foo > for loop := retry.Loop(spec); loop.Next(); { > result, err :=

Re: juju/retry take 2 - looping

2016-10-25 Thread Katherine Cox-Buday
Tim Penhey writes: > Forcing continue/break does not make it like all boring imperative > loops. I would like to understand this better. Here's why I think they're boring: - They're Go keywords. - They're present in many other languages and are well understood. - I

Re: juju/retry take 2 - looping

2016-10-25 Thread roger peppe
On 25 October 2016 at 22:40, Tim Penhey wrote: > On 26/10/16 10:30, Katherine Cox-Buday wrote: >> >> I think this is a hint that this is the wrong approach. The edge-cases >> begin showing the cracks in the abstraction and end up making the code more >> complex. Consider

Re: juju/retry take 2 - looping

2016-10-25 Thread Tim Penhey
On 26/10/16 10:30, Katherine Cox-Buday wrote: I think this is a hint that this is the wrong approach. The edge-cases begin showing the cracks in the abstraction and end up making the code more complex. Consider your example instead of: var finalResult Foo for loop := retry.Loop(spec);

Re: juju/retry take 2 - looping

2016-10-25 Thread Katherine Cox-Buday
Tim Penhey writes: > 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. I will argue here that this goes against a

Re: juju/retry take 2 - looping

2016-10-25 Thread Tim Penhey
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

Re: juju/retry take 2 - looping

2016-10-20 Thread roger peppe
On 20 October 2016 at 16:30, Katherine Cox-Buday wrote: > John Meinel 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

Re: juju/retry take 2 - looping

2016-10-20 Thread Katherine Cox-Buday
John Meinel 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

Re: juju/retry take 2 - looping

2016-10-20 Thread Ian Booth
I really like where the enhancements are headed. I feel they offer the syntax that some folks wanted, with the safety and validation of the initial implementation. Best of both worlds. On 20/10/16 13:09, Tim Penhey wrote: > Hi folks, > > https://github.com/juju/retry/pull/5/files > > As often

Re: juju/retry take 2 - looping

2016-10-20 Thread John Meinel
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 wonder if we want to push harder on always

juju/retry take 2 - looping

2016-10-19 Thread Tim Penhey
Hi folks, https://github.com/juju/retry/pull/5/files As often is the case, the pure solution is not always the best. What seemed initially like the best approach didn't end up that way. Both Katherine and Roger had other retry proposals that got me thinking about changes to the juju/retry