Add metadata exchange to trigger and handler #15043
Conversation
DavidOrchard
force-pushed
the
feature/CAPPL-81-metadata-exchange-3
branch
8 times, most recently
from
f2abb56 to
5117aae
Compare
DavidOrchard
force-pushed
the
feature/CAPPL-81-metadata-exchange-3
branch
from
5117aae to
e966e1e
Compare
DavidOrchard
force-pushed
the
feature/CAPPL-81-metadata-exchange-3
branch
from
e966e1e to
b9cdec8
Compare
| // TODO: Should we return here instead? | ||
| payloadJSON, _ = json.Marshal(ghcapabilities.TriggerResponsePayload{Status: "ERROR", ErrorMessage: fmt.Sprintf("error %s marshalling payload", err.Error())}) |
There was a problem hiding this comment.
I think we can log the error
| } | ||
| err = h.connector.SignAndSendToGateway(ctx, gatewayIDStr, &body) | ||
| if err != nil { | ||
| h.lggr.Errorw("error sending message", "gateway", gatewayIDStr, "err", err) |
There was a problem hiding this comment.
can return errors.Wrap(err, "error sending message") here and it will get logged in loop()
| count uint | ||
| } | ||
|
|
||
| func (h *handler) updateTriggerConsensus() { |
There was a problem hiding this comment.
looks like this function is not called in this PR. how do we determine when we should trigger consensus?
There was a problem hiding this comment.
Good question. I forgot to put a TODO that once i have a timer with tests working (from the trigger), the handler needs to do something similar. I've updated the PR description.
| @@ -38,6 +43,7 @@ type webapiTrigger struct { | |||
| ch chan<- capabilities.TriggerResponse | |||
| config webapicap.TriggerConfig | |||
| rateLimiter *common.RateLimiter | |||
| rawConfig *values.Map | |||
There was a problem hiding this comment.
what data is in this rawConfig?
There was a problem hiding this comment.
It's the config that is in the triggers config, ie allowedSenders, allowedTopics, RateLimiter.. I wanted to make the metadata exchange "future-proof" so that changes to the config don't require rewriting the exchange logic.
Metadata exchange from handlers to gateways.
TODOs before merge: