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.

Sign up for GitHub

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

Jump to bottom

[ResponseOps][Rules] Delete legacy routes #203148

Merged
merged 15 commits into from
Dec 16, 2024
Merged

[ResponseOps][Rules] Delete legacy routes #203148

merged 15 commits into from
Dec 16, 2024

Conversation

js-jankisalvi
Copy link
Contributor

@js-jankisalvi js-jankisalvi commented Dec 5, 2024 •
edited
Loading

Summary

Resolves #195179
Resolves #192558

This PR deletes deprecated legacy alerts routes api/alerts/alert in v9.0.
It also updates docs to reflect the same.

Checklist

  • Documentation was added for features that require explanation or tutorials

Release notes

Deleted deprecated alerts routes.

@js-jankisalvi js-jankisalvi added the Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) label Dec 5, 2024
@js-jankisalvi js-jankisalvi self-assigned this Dec 5, 2024
@js-jankisalvi js-jankisalvi added release_note:deprecation backport:skip This commit does not require backporting v9.0.0 labels Dec 12, 2024
@js-jankisalvi js-jankisalvi changed the title [ResponseOps][Rules] remove legacy routes [ResponseOps][Rules] Remove legacy routes Dec 12, 2024
@js-jankisalvi js-jankisalvi changed the title [ResponseOps][Rules] Remove legacy routes [ResponseOps][Rules] Delete legacy routes Dec 12, 2024
@js-jankisalvi js-jankisalvi marked this pull request as ready for review December 12, 2024 13:25
@js-jankisalvi js-jankisalvi requested review from a team as code owners December 12, 2024 13:25
@elasticmachine
Copy link
Contributor

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

@js-jankisalvi js-jankisalvi requested a review from lcawl December 12, 2024 13:25
consulthys
consulthys approved these changes Dec 12, 2024
View reviewed changes
Copy link
Contributor

@consulthys consulthys left a comment

Choose a reason for hiding this comment

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

LGT Stack Monitoring
(trivial change in functional tests)

@js-jankisalvi
Copy link
Contributor Author

@lcawl There are 2 apis still left in the bundle, so haven't updated the merge_ess_oas.js

@adcoelho
Copy link
Contributor

Should you also delete x-pack/plugins/alerting/docs/openapi/paths/api@alerting@rule@{ruleid}@_update_api_key.yaml ?

I think this one is also legacy and was deleted.

adcoelho
adcoelho approved these changes Dec 12, 2024
View reviewed changes
Copy link
Contributor

@adcoelho adcoelho left a comment

Choose a reason for hiding this comment

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

Approved except for the documentation comment I left above.

I think there will be a conflict with #204003

If I knew these would be deleted I wouldn't have done my changes 😅 but it's all good, they were small changes.

@js-jankisalvi
Copy link
Contributor Author

Should you also delete x-pack/plugins/alerting/docs/openapi/paths/api@alerting@rule@{ruleid}@_update_api_key.yaml ?

I think this one is also legacy and was deleted.

This is not a legacy rule. Legacy rules docs started with api@alerts@alert/@... or api@alerts@.... Above is @alerting@rule@{ruleid}

@kertal
Copy link
Member

kertal commented Dec 12, 2024

So this closes also #192558, right?

@adcoelho
Copy link
Contributor

@js-jankisalvi

This is not a legacy rule. Legacy rules docs started with api@alerts@alert/@... or api@alerts@.... Above is @alerting@rule@{ruleid}

Ah my bad, I saw x-pack/plugins/alerting/server/routes/legacy/update_api_key.ts and thought it was the same.

I already approved the PR anyway 👍

@js-jankisalvi
Copy link
Contributor Author

js-jankisalvi commented Dec 12, 2024 •
edited
Loading

So this closes also #192558, right?

I think yes, as I have updated LEGACY_BASE_ALERT_API_PATH in src/plugins/discover/public/application/view_alert/view_alert_utils.tsx to new api. Do you agree @cnasikas ?

@cnasikas
Copy link
Member

@js-jankisalvi @kertal Yes, this is correct!

@cnasikas cnasikas linked an issue Dec 12, 2024 that may be closed by this pull request
[Discover] Migrate from the legacy Get alert API to the new one #192558
Closed
lcawl
lcawl approved these changes Dec 12, 2024
View reviewed changes
Copy link
Contributor

@lcawl lcawl left a comment

Choose a reason for hiding this comment

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

API docs LGTM

cnasikas
cnasikas approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

🚀

kdelemme
kdelemme approved these changes Dec 13, 2024
View reviewed changes
Copy link
Contributor

@kdelemme kdelemme left a comment

Choose a reason for hiding this comment

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

ux-management code change LGTM

jughosta
jughosta approved these changes Dec 13, 2024
View reviewed changes
Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Data Discovery changes LGTM 👍

banderror
banderror approved these changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Change to x-pack/test/security_solution_api_integration/test_suites/detections_response/utils/alerts/wait_for_alert_to_complete.ts 👍

@js-jankisalvi js-jankisalvi enabled auto-merge (squash) December 16, 2024 13:09
@js-jankisalvi js-jankisalvi merged commit 279f4ae into elastic:main Dec 16, 2024
9 checks passed
@js-jankisalvi js-jankisalvi deleted the remove-deprecated-rule-apis branch December 16, 2024 14:35
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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
discover 786.1KB 786.1KB +1.0B

History

cc @js-jankisalvi

JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Dec 19, 2024
@js-jankisalvi @kibanamachine @JoseLuisGJ
[ResponseOps][Rules] Delete legacy routes (elastic#203148)
918177d 
## Summary

Resolves elastic#195179
Resolves elastic#192558

This PR deletes deprecated legacy alerts routes `api/alerts/alert` in
v9.0.
It also updates docs to reflect the same.


### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials



### Release notes
Deleted deprecated alerts routes.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Jan 13, 2025
@js-jankisalvi @kibanamachine @CAWilson94
[ResponseOps][Rules] Delete legacy routes (elastic#203148)
fa7eff0 
## Summary

Resolves elastic#195179
Resolves elastic#192558

This PR deletes deprecated legacy alerts routes `api/alerts/alert` in
v9.0.
It also updates docs to reflect the same.


### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials



### Release notes
Deleted deprecated alerts routes.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@consulthys consulthys consulthys approved these changes

@kdelemme kdelemme kdelemme approved these changes

@jughosta jughosta jughosta approved these changes

@adcoelho adcoelho adcoelho approved these changes

@banderror banderror banderror approved these changes

@cnasikas cnasikas cnasikas approved these changes

@lcawl lcawl lcawl approved these changes

Assignees

@js-jankisalvi js-jankisalvi

Labels
backport:skip This commit does not require backporting release_note:deprecation Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v9.0.0
Projects
None yet
Milestone
No milestone