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

[Synthetics] Refactor bulk delete monitor and params routes !! #195420

Merged
merged 26 commits into from
Nov 7, 2024

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Oct 8, 2024

Summary

Refactor bulk delete monitor and params routes !!

We need to remove usage for body from DELETE route.

Params

Params can be bulk delete now with POST request to /params/_bulk_delete endpoint

Monitors

Monitors can be bulk delete now with POST request to /monitors/_bulk_delete endpoint

@shahzad31 shahzad31 marked this pull request as ready for review October 9, 2024 10:11
@shahzad31 shahzad31 requested a review from a team as a code owner October 9, 2024 10:11
@shahzad31 shahzad31 added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes labels Oct 9, 2024
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Oct 9, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@@ -37,7 +37,7 @@ DELETE /api/synthetics/monitors/monitor1-id

==== Bulk Delete Monitors

You can delete multiple monitors by sending a list of config ids to a DELETE request to the `/api/synthetics/monitors` endpoint.
You can delete multiple monitors by sending a list of config ids to a POST request to the `/api/synthetics/monitors/_bulk_delete` endpoint.
Copy link
Contributor

Choose a reason for hiding this comment

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

Because these routes are publicly documented we will need to first deprecate the existing route and expose a the new /_bulk_delete route. And before we implement a deprecation we need to get it reviewed.

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that now that we aren't deprecating the api/synthetics/monitors route, but just crating a new _bulk_delete route, we do not need to do this. Is that correct?

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

Delete monitor looks good

Deleting a param leads to error

https://www.loom.com/share/59022fd92eb34f3b91076a8035fabc8e?sid=b22e242d-da29-4d26-a221-8d2aedb3f952

Error was 400 bad request.

{
    "statusCode": 400,
    "error": "Bad Request",
    "message": "[request body]: types that failed validation:\n- [request body.0.key]: expected value of type [string] but got [undefined]\n- [request body.1]: expected value of type [array] but got [Object]"
}

@elasticmachine
Copy link
Contributor

elasticmachine commented Nov 6, 2024

💚 Build Succeeded

  • Buildkite Build
  • Commit: 437d401
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-195420-437d401eb8fd

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
synthetics 1.1MB 1.1MB +130.0B

History

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM

@shahzad31 shahzad31 merged commit d25a2b4 into elastic:main Nov 7, 2024
26 checks passed
@shahzad31 shahzad31 deleted the delete-body branch November 7, 2024 09:51
@shahzad31 shahzad31 removed the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label Nov 7, 2024
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11720558634

@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Nov 7, 2024
@shahzad31 shahzad31 added backport:version Backport to applied version labels v8.17.0 v9.0.0 and removed backport:skip This commit does not require backporting v9.0.0 labels Nov 7, 2024
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11720567047

@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11720567048

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 7, 2024
…ic#195420)

## Summary

Refactor bulk delete monitor and params routes !!

We need to remove usage for body from DELETE route.

### Params

Params can be bulk delete now with POST request to
`/params/_bulk_delete` endpoint

### Monitors
Monitors can be bulk delete now with POST request to
`/monitors/_bulk_delete` endpoint

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

💚 All backports created successfully

Status Branch Result
8.x

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 7, 2024
…ic#195420)

## Summary

Refactor bulk delete monitor and params routes !!

We need to remove usage for body from DELETE route.

### Params

Params can be bulk delete now with POST request to
`/params/_bulk_delete` endpoint

### Monitors
Monitors can be bulk delete now with POST request to
`/monitors/_bulk_delete` endpoint

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

💚 All backports created successfully

Status Branch Result
8.x

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 7, 2024
…ic#195420)

## Summary

Refactor bulk delete monitor and params routes !!

We need to remove usage for body from DELETE route.

### Params

Params can be bulk delete now with POST request to
`/params/_bulk_delete` endpoint

### Monitors
Monitors can be bulk delete now with POST request to
`/monitors/_bulk_delete` endpoint

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

💚 All backports created successfully

Status Branch Result
8.x

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 Nov 7, 2024
…195420) (#199277)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Synthetics] Refactor bulk delete monitor and params routes !!
(#195420)](#195420)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-07T09:51:56Z","message":"[Synthetics]
Refactor bulk delete monitor and params routes !! (#195420)\n\n##
Summary\r\n\r\nRefactor bulk delete monitor and params routes !!
\r\n\r\nWe need to remove usage for body from DELETE route.\r\n\r\n###
Params\r\n\r\nParams can be bulk delete now with POST request
to\r\n`/params/_bulk_delete` endpoint\r\n\r\n### Monitors\r\nMonitors
can be bulk delete now with POST request to\r\n`/monitors/_bulk_delete`
endpoint","sha":"d25a2b442aff8d3b4559832257b98c0f50de922b","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","ci:project-deploy-observability","Team:obs-ux-management"],"title":"[Synthetics]
Refactor bulk delete monitor and params routes
!!","number":195420,"url":"https://github.com/elastic/kibana/pull/195420","mergeCommit":{"message":"[Synthetics]
Refactor bulk delete monitor and params routes !! (#195420)\n\n##
Summary\r\n\r\nRefactor bulk delete monitor and params routes !!
\r\n\r\nWe need to remove usage for body from DELETE route.\r\n\r\n###
Params\r\n\r\nParams can be bulk delete now with POST request
to\r\n`/params/_bulk_delete` endpoint\r\n\r\n### Monitors\r\nMonitors
can be bulk delete now with POST request to\r\n`/monitors/_bulk_delete`
endpoint","sha":"d25a2b442aff8d3b4559832257b98c0f50de922b"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195420","number":195420,"mergeCommit":{"message":"[Synthetics]
Refactor bulk delete monitor and params routes !! (#195420)\n\n##
Summary\r\n\r\nRefactor bulk delete monitor and params routes !!
\r\n\r\nWe need to remove usage for body from DELETE route.\r\n\r\n###
Params\r\n\r\nParams can be bulk delete now with POST request
to\r\n`/params/_bulk_delete` endpoint\r\n\r\n### Monitors\r\nMonitors
can be bulk delete now with POST request to\r\n`/monitors/_bulk_delete`
endpoint","sha":"d25a2b442aff8d3b4559832257b98c0f50de922b"}}]}]
BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
mbondyra pushed a commit to mbondyra/kibana that referenced this pull request Nov 8, 2024
…ic#195420)

## Summary

Refactor bulk delete monitor and params routes !! 

We need to remove usage for body from DELETE route.

### Params

Params can be bulk delete now with POST request to
`/params/_bulk_delete` endpoint

### Monitors
Monitors can be bulk delete now with POST request to
`/monitors/_bulk_delete` endpoint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants