-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[RAM] adds bulkUpdatesSchedules method to Task Manager API #132637
Changes from all commits
2759c2e
d24c2f3
ff55e01
6efc5fb
655a753
74c5a7c
7e1e7c7
815854b
79fc692
8197409
e9d4426
51469cc
0b96f46
9f1ccf7
edc7890
d20f2fd
f572388
f2bc696
0b44ef1
cb5411a
bdeadf6
0563f88
bd36d9a
64e7adb
6fb6bee
fc2f119
eb4af87
0e2d63a
52b794a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -211,7 +211,10 @@ export interface FindOptions extends IndexType { | |
filter?: string; | ||
} | ||
|
||
export type BulkEditFields = keyof Pick<Rule, 'actions' | 'tags'>; | ||
export type BulkEditFields = keyof Pick< | ||
Rule, | ||
'actions' | 'tags' | 'schedule' | 'throttle' | 'notifyWhen' | ||
>; | ||
|
||
export type BulkEditOperation = | ||
| { | ||
|
@@ -223,25 +226,23 @@ export type BulkEditOperation = | |
operation: 'add' | 'set'; | ||
field: Extract<BulkEditFields, 'actions'>; | ||
value: NormalizedAlertAction[]; | ||
} | ||
| { | ||
operation: 'set'; | ||
field: Extract<BulkEditFields, 'schedule'>; | ||
value: Rule['schedule']; | ||
} | ||
| { | ||
operation: 'set'; | ||
field: Extract<BulkEditFields, 'throttle'>; | ||
value: Rule['throttle']; | ||
} | ||
| { | ||
operation: 'set'; | ||
field: Extract<BulkEditFields, 'notifyWhen'>; | ||
value: Rule['notifyWhen']; | ||
}; | ||
|
||
// schedule, throttle, notifyWhen is commented out before https://github.com/elastic/kibana/issues/124850 will be implemented | ||
// | { | ||
// operation: 'set'; | ||
// field: Extract<BulkEditFields, 'schedule'>; | ||
// value: Rule['schedule']; | ||
// } | ||
// | { | ||
// operation: 'set'; | ||
// field: Extract<BulkEditFields, 'throttle'>; | ||
// value: Rule['throttle']; | ||
// } | ||
// | { | ||
// operation: 'set'; | ||
// field: Extract<BulkEditFields, 'notifyWhen'>; | ||
// value: Rule['notifyWhen']; | ||
// }; | ||
|
||
type RuleParamsModifier<Params extends RuleTypeParams> = (params: Params) => Promise<Params>; | ||
|
||
export interface BulkEditOptionsFilter<Params extends RuleTypeParams> { | ||
|
@@ -1494,6 +1495,36 @@ export class RulesClient { | |
); | ||
}); | ||
|
||
// update schedules only if schedule operation is present | ||
const scheduleOperation = options.operations.find( | ||
( | ||
operation | ||
): operation is Extract<BulkEditOperation, { field: Extract<BulkEditFields, 'schedule'> }> => | ||
operation.field === 'schedule' | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what other operations should affect rescheduling of tasks? Should There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only logic that causes a rule's task to get rescheduled at this time is when the rule interval changes. We should be ok to skip the other operations. |
||
|
||
if (scheduleOperation?.value) { | ||
const taskIds = updatedRules.reduce<string[]>((acc, rule) => { | ||
if (rule.scheduledTaskId) { | ||
acc.push(rule.scheduledTaskId); | ||
} | ||
return acc; | ||
}, []); | ||
|
||
try { | ||
await this.taskManager.bulkUpdateSchedules(taskIds, scheduleOperation.value); | ||
this.logger.debug( | ||
`Successfully updated schedules for underlying tasks: ${taskIds.join(', ')}` | ||
); | ||
} catch (error) { | ||
this.logger.error( | ||
`Failure to update schedules for underlying tasks: ${taskIds.join( | ||
', ' | ||
)}. TaskManager bulkUpdateSchedules failed with Error: ${error.message}` | ||
); | ||
} | ||
} | ||
|
||
return { rules: updatedRules, errors, total }; | ||
} | ||
|
||
|
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.
Currently functional tests are skipped due to #132195
I plan to fix them and add additional tests in the following PR, so won't mix everything together.
I can do it in this PR, if it preferable
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.
For reviewers:
I believe this is the follow up PR @vitaliidm is referring to 😉
https://github.com/elastic/kibana/pull/133635/files
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'd prefer to see them here, but TBH doesn't make sense if they're skipped, so ok in the follow-up. I added a comment to that PR as a reminder.