V2 #26
V2 #26
Conversation
Remove the Listener implementation in the interest of doing one thing and doing it well. We can add it back in another package down the line.
README.md
Outdated
| @@ -1,18 +1,41 @@ | |||
| # retry | |||
|
|
|||
| An expressive, flexible retry package for Go. | |||
| A small retry package for Go. | |||
There was a problem hiding this comment.
This doesn't really tell me what retry means in this context.
Neither did the old statement, but 🤷♀️
README.md
Outdated
| - Backoff helper | ||
| - Retrying net.Listener wrapper | ||
| ## Features | ||
| - For loop experience instead of closures |
There was a problem hiding this comment.
"For" should probably be in backticks to indicate the keyword
README.md
Outdated
| r := retry.New(time.Second, time.Second*10) | ||
| for r.Wait(ctx) { | ||
| _, err := http.Get("https://google.com") | ||
| r.SetError(err) |
There was a problem hiding this comment.
Why do we need a set error function? Couldn't we return if there's an error? If there's no error, the user would break.
What's the advantage over using a ticker? Is it the maximum retries?
retrier.go
Outdated
| "time" | ||
| ) | ||
|
|
||
| // Retrier represents a retry instance. |
There was a problem hiding this comment.
Not indicative of what this does. There is no "retry" instance.
| err *error | ||
| } | ||
|
|
||
| // New creates a retrier that exponentially backs off from floor to ceil pauses. |
We can always add it back later.
| func pingGoogle(ctx context.Context) error { | ||
| var err error | ||
| r := retry.New(time.Second, time.Second*10) | ||
| for r.Wait(ctx) { |
There was a problem hiding this comment.
lmfao just realized this was the API from before
| second. | ||
| ```go | ||
| func pingGoogle(ctx context.Context) error { | ||
| var err error |
| if err != nil { | ||
| continue | ||
| } | ||
| break |
| r.delay = r.ceil | ||
| } | ||
| select { | ||
| case <-time.After(r.delay): |
There was a problem hiding this comment.
Might be a good idea to use time.NewTimer here to avoid memory usage when things are failing consistently and contexts are being cancelled.
| } | ||
| break | ||
| } | ||
| return err |
| for n := 0; r.Wait(ctx) && n < 10; n++ { | ||
| _, err = http.Get("https://google.com") | ||
| if err != nil { | ||
| continue |
There was a problem hiding this comment.
Then you don't need to declare err above either. Or need a break/return below.
Errortypeerr == io.EOF)Listenerfloor == ceil. Also: a constant sleep obviates much of the point of using a retry package in the first place.I didn't bother updating the major version since the import path was already updated following our new organization.