-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] System action in bulk delete #170741
[RAM] System action in bulk delete #170741
Conversation
Pinging @elastic/response-ops (Team:ResponseOps) |
x-pack/plugins/alerting/server/application/rule/methods/bulk_delete/bulk_delete_rules.test.ts
Show resolved
Hide resolved
x-pack/plugins/alerting/server/data/rule/types/rule_attributes.ts
Outdated
Show resolved
Hide resolved
} catch (e) { | ||
context.logger.warn(`Error validating bulk enabled rule domain object for id: ${id}, ${e}`); | ||
} | ||
return ruleDomain; |
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 can avoid the two loop if just do that, correct?
return ruleDomain; | |
return transformRuleDomainToRule<Params>(ruleDomain); |
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've maid this change on bulk enable
PR: bb43e3fa3074c88c0a86899f3acae71873e973c2
It'll appear here after merge
try { | ||
ruleDomainSchema.validate(ruleDomain); | ||
} catch (e) { | ||
context.logger.warn(`Error validating bulk enabled rule domain object for id: ${id}, ${e}`); | ||
} |
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 will ask @JiaweiWu about that but it is weird to validate and do nothing about it. Let's keep it for now but it is weird
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.
What was the reason behind that?
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.
it was safe guard to make sure that we are dealing with the right type
💔 Build FailedFailed CI Steps
Test Failures
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @guskovaue |
Fix: #170392
Meta: #160367
Summary
This PR enables system actions for the Bulk Delete Rule API.
Checklist