Skip to content
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] replaces savedObjectClient.bulkUpdate with bulkCreate method in RulesClient.bulkEdit #137593

Merged

Conversation

vitaliidm
Copy link
Contributor

@vitaliidm vitaliidm commented Jul 29, 2022

Summary

savedObjectClient.bulkUpdate method uses ES _bulk API with update action, which doesn’t allow to delete fields from Saved Objects(SO), i.e. overwrite them with undefined
https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html

Instead, in this PR it replaced by savedObjectClient.bulkCreate with overwrite=true, that allows to remove existing fields by complete overwrite of SOs. It results in _bulk API query index action

Checklist

Delete any items that are not applicable to this PR.

@vitaliidm vitaliidm self-assigned this Jul 29, 2022
@vitaliidm vitaliidm added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesManagement Issues related to the Rules Management UX backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes labels Jul 29, 2022
@vitaliidm
Copy link
Contributor Author

Simple example with _bulk update/index actions on SO(can be run in Kibana dev tools)

// create index
PUT /timeline-issue
{
  "settings": {
    "number_of_shards": 1
  },
  "mappings": {
    "properties": {
      "params": {  "type": "flattened" }
    }
  }
}

// add new SO
POST /timeline-issue/_doc/1
{
  "params": {
    "description": "test SO 1",
    "timelineTitle": "Test TT"
  }
}

// rewrite values in SO by using _bulk, see timelineTitle changed to new value
POST /_bulk?refresh=wait_for&_source=true
{"update":{"_id":"1","_index":"timeline-issue"}}
{"doc":{"params":{"description": "test SO 1","timelineTitle": "Test TT Bulk"}}}

// rewrite values in SO by using _bulk, see timelineTitle has not benn rewritten
POST /_bulk?refresh=wait_for&_source=true
{"update":{"_id":"1","_index":"timeline-issue"}}
{"doc":{"params":{"description": "test SO 2"}}}

// simple put on contrary rewrites field if it's undefined
PUT /timeline-issue/_doc/1
{
  "params": {
    "description": "test SO 1"
  }
}

// it's possible to remove fields by using "index" action
GET /timeline-issue/_doc/1
POST /_bulk?refresh=wait_for&_source=true
{"index":{"_id":"1","_index":"timeline-issue"}}
{"doc":{"params":{"description": "test SO 5"}}}

@vitaliidm vitaliidm changed the title [RAM] move to bulkCreate in RulesClient.bulkEdit [RAM] replaces savedObjectClient.bulkUpdate with bulkCreate method in RulesClient.bulkEdit Jul 29, 2022
@vitaliidm vitaliidm marked this pull request as ready for review July 29, 2022 15:54
@vitaliidm vitaliidm requested a review from a team as a code owner July 29, 2022 15:54
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @vitaliidm

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vitaliidm vitaliidm merged commit ff062ce into elastic:main Aug 3, 2022
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 3, 2022
… RulesClient.bulkEdit (elastic#137593)

## Summary

`savedObjectClient.bulkUpdate` method  uses ES _bulk API with `update` action, which doesn’t allow to delete fields from Saved Objects(SO), i.e. overwrite them with undefined
https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html

Instead, in this PR it replaced by `savedObjectClient.bulkCreate` with `overwrite=true`, that allows to remove existing fields by complete overwrite of SOs. It results in _bulk API query `index` action

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

(cherry picked from commit ff062ce)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.4

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Aug 3, 2022
… RulesClient.bulkEdit (#137593) (#137976)

## Summary

`savedObjectClient.bulkUpdate` method  uses ES _bulk API with `update` action, which doesn’t allow to delete fields from Saved Objects(SO), i.e. overwrite them with undefined
https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html

Instead, in this PR it replaced by `savedObjectClient.bulkCreate` with `overwrite=true`, that allows to remove existing fields by complete overwrite of SOs. It results in _bulk API query `index` action

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

(cherry picked from commit ff062ce)

Co-authored-by: Vitalii Dmyterko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) backport Feature:Alerting/RulesManagement Issues related to the Rules Management UX refactoring release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.4.0 v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants