app/retry: implement async retryer #346
Conversation
Codecov Report
@@ Coverage Diff @@
## main #346 +/- ##
==========================================
+ Coverage 55.36% 55.68% +0.32%
==========================================
Files 60 62 +2
Lines 5175 5332 +157
==========================================
+ Hits 2865 2969 +104
- Misses 1917 1962 +45
- Partials 393 401 +8
Continue to review full report at Codecov.
|
| 00:00 [35mDEBG[0m msg1 {"ctx1": 1, "caller": "log_test.go:44"} | ||
| 00:00 [34mINFO[0m msg2 {"ctx2": 2, "wrap2": 2, "caller": "log_test.go:45"} | ||
| 00:00 [33mWARN[0m msg3a {"wrap3": "a", "wrap2": 2, "caller": "log_test.go:46"} | ||
| 00:00 [33mWARN[0m msg3b {"wrap3": "b", "wrap2": 2, "caller": "log_test.go:47"} |
There was a problem hiding this comment.
TODO(corver): Create ticket to stub caller lines from golden files to make it less brittle
There was a problem hiding this comment.
I dont understand that sentence, what are golden files? and stub caller lines?
There was a problem hiding this comment.
See here for golden files: https://github.com/sebdah/goldie
This is a caller line: "caller": "log_test.go:47". The line number changes when the test file code changes. That makes the test brittle. Since the logic didn't change.
app/retry/retry.go
Outdated
| // It is intended to be used asynchronously: | ||
| // go retryer.DoAsync(ctx, duty.Slot, "foo", fn) | ||
| func (r *Retryer) DoAsync(parent context.Context, slot int64, name string, fn func(context.Context) error) { | ||
| defer r.wrapActive()() |
There was a problem hiding this comment.
@corverroos I don't understand what this is doing, because it looks like it will do both calls on defer. Also, it seems that this can be done by the library sync.WaitingGroup
There was a problem hiding this comment.
Yeah, this is a pattern to wrap a function in some logic. It is called at the start and end of a function. It increments and decrements the active counter.
There was a problem hiding this comment.
Cool, changed it to sync.Waitgroup.
There was a problem hiding this comment.
Are you sure, I think it is being called both things at the end of the funciton, like defer wraps both calls. Isn't? @corverroos
There was a problem hiding this comment.
@corverroos I think you didn't push the change to WaitingGroup
There was a problem hiding this comment.
yeah sorry, pushed now
| 00:00 [35mDEBG[0m msg1 {"ctx1": 1, "caller": "log_test.go:44"} | ||
| 00:00 [34mINFO[0m msg2 {"ctx2": 2, "wrap2": 2, "caller": "log_test.go:45"} | ||
| 00:00 [33mWARN[0m msg3a {"wrap3": "a", "wrap2": 2, "caller": "log_test.go:46"} | ||
| 00:00 [33mWARN[0m msg3b {"wrap3": "b", "wrap2": 2, "caller": "log_test.go:47"} |
There was a problem hiding this comment.
I dont understand that sentence, what are golden files? and stub caller lines?
Package retry provides a generic async slot function executor with retries for robustness against network failures.
Functions are linked to a slot, executed asynchronously and network or context errors retried with backoff
until duties related to a slot have elapsed (5 slots later).
category: feature
ticket: #354