Add DefinitionBatchActions to execute after a batch of system definitions #462
Conversation
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Codecov Report
@@ Coverage Diff @@
## main #462 +/- ##
=======================================
Coverage 99.92% 99.92%
=======================================
Files 266 266
Lines 15101 15133 +32
=======================================
+ Hits 15090 15122 +32
Misses 11 11
Continue to review full report at Codecov.
|
…ions The bulk of the processing for a system definition broadcast occurs synchronously, within a database RunAsGroup call. For definitions that trigger calls out of FireFly (ie via plugins), there must be an option to trigger steps outside RunAsGroup (and possibly another final set of steps inside RunAsGroup). Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
peterbroadhurst
left a comment
peterbroadhurst
left a comment
There was a problem hiding this comment.
This is great - thanks for finding what looks like a pretty elegant solution to cracking this tough nut.
My only comments, are on your comments - and about being able to understand how to use the feature in the future within the code.
| ActionWait | ||
| ) | ||
|
|
||
| // DefinitionBatchActions are actions to be taken at the end of a definition batch - should only run if batch is successfully processed |
There was a problem hiding this comment.
Maybe worth mentioning that these callbacks are allowed to share state, by using a closure.
e.g. PreFinalize can contain idempotent logic that reads/updates the database, and calculates some kind of outcome, and then Finalize can write that outcome by referencing the variable from the closure.
| // DefinitionBatchActions are actions to be taken at the end of a definition batch - should only run if batch is successfully processed | ||
| type DefinitionBatchActions struct { | ||
| // PreFinalize may perform a blocking action (possibly to an external connector) that should execute outside database RunAsGroup | ||
| PreFinalize func(ctx context.Context) error |
There was a problem hiding this comment.
Worth noting that error returned from here, will cause the whole batch to "block" (indefinitely retry), just like ActionRetry would from HandleDefinitionBroadcast.
There is not (currently) any option to return an ActionReject in the PreFinalize phase
There was a problem hiding this comment.
... after I wrote this, I found it's actually covered in the internal/events/aggregator comments - not sure if one should refer to the other, to avoid two places that we might forget to update the rules on.
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
3 participants
Resolves #463
Work still pending:
Remaining test coverageFinalizepatternIf the foundation looks good, I'll add on these remaining pieces.