[Response Ops][Alerting] Refactor ExecutionHandler stage 1
#186666
Conversation
ymao1
force-pushed
the
refactor-execution-handler-stage-1
branch
from
dbeb39e
to
110d9e2
Compare
| ); | ||
| this.previousStartedAt = context.previousStartedAt; | ||
|
|
||
| for (const [_, scheduler] of Object.entries(schedulers)) { |
There was a problem hiding this comment.
This grabs every scheduler from the schedulers/index.ts file
There was a problem hiding this comment.
I think the schedulers calculation, and sorting, can probably be done in static code, not even in the constructor. Minor perf improvement :-)
There was a problem hiding this comment.
I tried moving schedulers into a static variable
static schedulers: Array<
IActionScheduler<State, Context, ActionGroupIds, RecoveryActionGroupId>
> = [];
but I get a TS warning: Static members cannot reference class type parameters
| } | ||
|
|
||
| // sort schedulers by priority | ||
| this.schedulers.sort((a, b) => a.priority - b.priority); |
There was a problem hiding this comment.
and then sorts them by self-identified priority
| }); | ||
|
|
||
| const executables = []; | ||
| for (const scheduler of this.schedulers) { |
There was a problem hiding this comment.
This is the only change to the run function. Previously, it called
const executables = await this.generateExecutables(alerts, throttledSummaryActions);
| @@ -51,100 +47,16 @@ const actionsClient = actionsClientMock.create(); | |||
| const alertsClient = alertsClientMock.create(); | |||
| const mockActionsPlugin = actionsMock.createStart(); | |||
| const apiKey = Buffer.from('123:abc').toString('base64'); | |||
| const ruleType: NormalizedRuleType< | |||
There was a problem hiding this comment.
moved these to a test_fixtures file
| queryOptions: GetSummarizedAlertsParams; | ||
| } | ||
|
|
||
| export const getSummarizedAlerts = async < |
There was a problem hiding this comment.
this was a private function in ExecutionHandler class. Moved to a reusable helper function
x-pack/plugins/alerting/server/task_runner/action_scheduler/rule_action_helper.ts
Show resolved
Hide resolved
ymao1
changed the title
ExecutionHandler stage 1
ymao1
added
Feature:Alerting
Team:ResponseOps
|
Pinging @elastic/response-ops (Team:ResponseOps) |
|
@elasticmachine merge upstream |
x-pack/plugins/alerting/server/task_runner/action_scheduler/rule_action_helper.ts
Show resolved
Hide resolved
…ion-handler-stage-1
| ); | ||
| this.previousStartedAt = context.previousStartedAt; | ||
|
|
||
| for (const [_, scheduler] of Object.entries(schedulers)) { |
There was a problem hiding this comment.
I think the schedulers calculation, and sorting, can probably be done in static code, not even in the constructor. Minor perf improvement :-)
…ion-handler-stage-1
💔 Build Failed
Failed CI Steps
Metrics [docs]Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
cc @ymao1 |
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @ymao1 |
Resolves #186533
Summary
Stage 1 of
ExecutionHandlerrefactor:ExecutionHandlertoActionScheduler.SummaryActionScheduler,SystemActionScheduler,PerAlertActionScheduler)ExecutionHandler.generateExecutablesfunction into the appropriate action type class and combine the returned executables from each scheduler class.GH is not recognizing the rename from
ExecutionHandlertoActionSchedulerso I've called out the primary difference between the two files (other than the rename) which is to get the executables from each scheduler class instead of from agenerateExecutablesfunction. Removed thegenerateExecutablesfn from theActionSchedulerand any associated private helper functions.