-
Notifications
You must be signed in to change notification settings - Fork 920
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
feat(chord): ErrorCallback to handle Group failures #605
base: master
Are you sure you want to change the base?
Conversation
* unit tests for processChord + interfaces + mockery * suite + successChord test * handle cases where chords are not present * logic fixes for signature/chord * amqp test
cc @zeroshade |
Codecov Report
@@ Coverage Diff @@
## master #605 +/- ##
==========================================
- Coverage 40.86% 39.05% -1.81%
==========================================
Files 34 32 -2
Lines 2763 2929 +166
==========================================
+ Hits 1129 1144 +15
- Misses 1490 1641 +151
Partials 144 144
Continue to review full report at Codecov.
|
A chord is a Group and a task that executes when the Group is done with the results of the group.
If a task fails in that group - nothing gets called at the end of the group execution.
Proposal:
NewChordWithError
which would make aChord(Group, SuccessTask, FailureTask)
using the following logic:The FailureTask would take an aggregate of errors of all the failed tasks e.g.
func FailureTask(errs []string) error
Any successful completions of the tasks in the group would be 'dropped'
Changes in this PR
tasks.NewChordWithError()
function that takes an additionalSignature
tasks.NewChord
maintains backwards compatiblityresults.AsyncChordResult
behavior modified to fetch ErrorCallback results if one of the tasks failsworker.go
modifications for processingErrorChord
processChord
Server
andWorker
Note: Unit tests made this a larger change as they required making interfaces out of Server and Worker and injecting those into processing functions. I'd love to take this effort further - but I wanted to keep this change as small as possible. It's already 16 files so I somewhat failed at that.
Let me know if you have thoughts on the utility of the change, etc