-
Notifications
You must be signed in to change notification settings - Fork 16
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
[CAPPL-270/271] Fix Consensus bugs #934
Conversation
cedric-cordenier
commented
Nov 12, 2024
- Fix "result is not a pointer error" in the reduce aggregator
- Continue rather than error if we encounter an aggregation error
- Fix "result is not a pointer error" in the reduce aggregator - Continue rather than error if we encounter an aggregation error
30713b4
to
6bd5edd
Compare
@@ -517,22 +582,6 @@ func TestReduceAggregator_Aggregate(t *testing.T) { | |||
return map[commontypes.OracleID][]values.Value{} | |||
}, | |||
}, | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this because this isn't actually a problem in the current implementation and doesn't error
@@ -339,7 +339,7 @@ func (r *reportingPlugin) Outcome(ctx context.Context, outctx ocr3types.OutcomeC | |||
outcome, err2 := agg.Aggregate(lggr, workflowOutcome, obs, r.config.F) | |||
if err2 != nil { | |||
lggr.Errorw("error aggregating outcome", "error", err2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: it would make sense to log weid here and all logs above (can be done separately)