Add a shared actions context structure #5182
Conversation
There was a problem hiding this comment.
How do we see this aggregation working? To quote from #5119 (comment)
If license scanning takes 100ms and static analysis takes 5 minutes, do we add the license scan report right away and then update the comment later when static analysis comes in?
I'm having a little trouble visualizing how this works -- I think this assumes that all the rule eval and actions for a given event happen on the same Minder server? And we'll end up with something on the event-handling context that allows "enqueue this to be done in the future"?
Thinking about audit logs and splitting action execution from action computation, I'd love to have a data structure for collecting these actions, rather than a list of functions. Ideallly, this would look like:
{notification / reminder edge} --> [visit $foo resource]
[visit $foo resource] -> (per-rule) {rule-eval} -> (per-rule) [eval results] & [action plan?]
[action plan] -> {action-processor} -> [action result]
[eval results] -> {audit-log}
[action result] -> {audit-log}
Where [message]s are in brackets, and {computation} is in braces.
| @@ -138,21 +138,23 @@ func (e *executor) EvalEntityEvent(ctx context.Context, inf *entities.EntityInfo | |||
| return fmt.Errorf("error while retrieving profiles and rule instances: %w", err) | |||
| } | |||
|
|
|||
| sacctx, sac := actions.WithSharedActionsContext(ctx) | |||
There was a problem hiding this comment.
Why not still use ctx for sacctx?
Right, it assumes an set of profiles will be evaluated for one event in a single Minder server. As is the case today.
I was thinking of kicking it off immediately for now. We probably need to add such mechanism later when the use-case comes.
The eval params gets us partly there, the main missing piece is the action results. Let me give that part some thought. |
JAORMX
force-pushed
the
shared-actions-context
branch
from
b403b5c to
9fa39a6
Compare
This shared space will allow actions to gather structures or results that will allow it to aggregate and flush. In more solid terms, we'd be able to aggregate PR comments, iterate them, and issue just one big comment. Signed-off-by: Juan Antonio Osorio <[email protected]>
JAORMX
force-pushed
the
shared-actions-context
branch
from
9fa39a6 to
551769c
Compare
Summary
This shared space will allow actions to gather structures or results
that will allow it to aggregate and flush. In more solid terms, we'd be
able to aggregate PR comments, iterate them, and issue just one big
comment.
The intention is for drivers (e.g. the pull request comment alert type) to fetch this from the context and push shared statements (e.g. comments), plus an aggregation handling function that meets the relevant signature. The aggregation function will be called in the end, thus allowing it to gather all comments and publish just one result.
Fixes #5119
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: