Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: make default unit nanosecond #7

Closed
zaquestion opened this issue Apr 11, 2018 · 4 comments
Closed

Proposal: make default unit nanosecond #7

zaquestion opened this issue Apr 11, 2018 · 4 comments

Comments

@zaquestion
Copy link

Its somewhat non-intuitive when setting a delay which takes a time.Duration that the units are in microseconds. Its easy to write code like this and think its valid.

       err = retry.Do(func() error {
               return foo()
       }, retry.Attempts(3), retry.Delay(time.Second))
@codyaray
Copy link

+💯

This is a huge bug in the usability of this library.

I just spent a couple hours tracking down why my app was appearing to just hang. My configuration of retry.Attempts(5), retry.Delay(1*time.Minute) was calling time.Sleep for 16h40m0s.

See https://play.golang.org/p/hqx9XFcPATj

A time.Duration is represented in nanoseconds. The fact that this library multiplies by microseconds by default completely breaks the standard time math in golang.

@JaSei
Copy link
Collaborator

JaSei commented Nov 27, 2018

Good point, thank you... What you mean about change API to delay and unit merge to one attribute delay which will be parsed by time.ParseDuration ?

@codyaray
Copy link

I think the api should just take a delay parameter of type time.Duration and there shouldn't be a separate unit field.

@JaSei
Copy link
Collaborator

JaSei commented Dec 3, 2018

#11

@JaSei JaSei closed this as completed Dec 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants