Add the ability to specify MaxDuration instead of Attempts. #2

Merged
merged 6 commits into from Oct 29, 2015

Conversation

Projects
None yet
3 participants
Owner

howbazaar commented Oct 27, 2015

During the initial work to add to juju/juju, it became apparent that being able to specify a MaxDuration rather than an Attempt count is desirable.

README.md
@@ -222,6 +204,13 @@ type CallArgs struct {
// value is specified there is no maximum delay.
MaxDelay time.Duration
+ // MaxWait specifies the maximum time the Call function should wait. The
+ // wait time is the summation of the delays over the attempts. If the next
@mjs

mjs Oct 28, 2015

nit: "running total" might be clearer than "summation"

README.md
+ // delay time would take the total waiting duration over MaxWait, then an
+ // WaitTimeExceeded error is returned. If no value is specified, Call will
+ // continue until the number of attempts is complete.
+ MaxWait time.Duration
@mjs

mjs Oct 28, 2015

The concept of MaxWait seems a little strange to me. If I understand it correctly it's compared against the total time spent waiting in between attempts. This seems like a counter-intuitive parameter.

What about having MaxDuration? Where the caller can set the maximum time the whole process can take (including the time taken to call Func etc).

@howbazaar howbazaar changed the title from Add the ability to specify MaxWait instead of Attempts. to Add the ability to specify MaxDuration instead of Attempts. Oct 29, 2015

retry_test.go
-func (*mockClock) Now() time.Time {
- return time.Now()
+func (mock *mockClock) Now() time.Time {
+ if mock.now.IsZero() {
@mjs

mjs Oct 29, 2015

Is this actually necessary? The clock could just start at zero couldn't it?

mjs commented Oct 29, 2015

LGTM!

Owner

howbazaar commented Oct 29, 2015

$$merge$$

Contributor

jujubot commented Oct 29, 2015

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju-retry

jujubot added a commit that referenced this pull request Oct 29, 2015

Merge pull request #2 from howbazaar/max-wait
Add the ability to specify MaxDuration instead of Attempts.

During the initial work to add to juju/juju, it became apparent that being able to specify a MaxDuration rather than an Attempt count is desirable.

@jujubot jujubot merged commit 62c6203 into juju:master Oct 29, 2015

@howbazaar howbazaar deleted the howbazaar:max-wait branch Oct 29, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment