-
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 1 commit
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 |
---|---|---|
|
@@ -12,6 +12,10 @@ import { ILicenseState, RuleTypeDisabledError } from '../lib'; | |
import { verifyAccessAndContext, rewriteRule, handleDisabledApiKeysError } from './lib'; | ||
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types'; | ||
|
||
const scheduleSchema = schema.object({ | ||
interval: schema.string(), | ||
}); | ||
|
||
const ruleActionSchema = schema.object({ | ||
group: schema.string(), | ||
id: schema.string(), | ||
|
@@ -34,6 +38,11 @@ const operationsSchema = schema.arrayOf( | |
field: schema.literal('actions'), | ||
value: schema.arrayOf(ruleActionSchema), | ||
}), | ||
schema.object({ | ||
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. Currently functional tests are skipped due to #132195 I can do it in this PR, if it preferable 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. For reviewers: 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. 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. |
||
operation: schema.literal('set'), | ||
field: schema.literal('schedule'), | ||
value: scheduleSchema, | ||
}), | ||
]), | ||
{ minSize: 1 } | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -211,7 +211,7 @@ export interface FindOptions extends IndexType { | |
filter?: string; | ||
} | ||
|
||
export type BulkEditFields = keyof Pick<Rule, 'actions' | 'tags'>; | ||
export type BulkEditFields = keyof Pick<Rule, 'actions' | 'tags' | 'schedule'>; | ||
|
||
export type BulkEditOperation = | ||
| { | ||
|
@@ -223,6 +223,11 @@ export type BulkEditOperation = | |
operation: 'add' | 'set'; | ||
field: Extract<BulkEditFields, 'actions'>; | ||
value: NormalizedAlertAction[]; | ||
} | ||
| { | ||
operation: 'set'; | ||
field: Extract<BulkEditFields, 'schedule'>; | ||
value: Rule['schedule']; | ||
}; | ||
|
||
// schedule, throttle, notifyWhen is commented out before https://github.com/elastic/kibana/issues/124850 will be implemented | ||
|
@@ -1494,6 +1499,32 @@ export class RulesClient { | |
); | ||
}); | ||
|
||
// update schedules only if schedule operation is present | ||
const scheduleOperation = options.operations.find( | ||
(op): op is Extract<BulkEditOperation, { field: Extract<BulkEditFields, 'schedule'> }> => | ||
op.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); | ||
} catch (error) { | ||
this.auditLogger?.log( | ||
ymao1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ruleAuditEvent({ | ||
action: RuleAuditAction.BULK_EDIT, | ||
error, | ||
}) | ||
); | ||
} | ||
} | ||
|
||
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.
We have a schema config validator we use for intervals, as seen here:
kibana/x-pack/plugins/alerting/server/routes/create_rule.ts
Line 36 in 5c166a6
Using that, a bad interval sent will "fail fast" and not somewhere else, deeper in the framework.
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.
thanks, I've replaced it with
validateDurationSchema