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

Create new rule HTTP APIs #93980

Merged

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented Mar 8, 2021

⚠️ This PR merges into a feature branch

Part of #93977

Note: To keep this PR small, I left out the doc changes and the API integration tests. These two steps are noted in #93977 and will be done in follow-up PRs.

Note for reviewers

This PR is huge because it's mostly copy/paste from the legacy APIs, apply the new route/terminology, etc. The following guidelines will facilitate the review since most of the changes are from copy/paste in the routes folder.

  • Routes: The route files are a copy-paste from their equivalent in /legacy folder while renaming the file names to something more route specific. IF you copy the content of the file over the legacy equivalent, you will see the changes I've applied on top. (ex: copy contents from create_rule.ts into legacy/create,ts and the diff in git will show what I've changed).
  • Unit tests: The *.test.ts files are a copy-paste from their equivalent in /legacy folder. If you copy the content of the test file over the legacy equivalent, you will see the changes I've applied on top. (ex: copy contents from create_rule.test.ts into legacy/create.test.ts and the diff in git will show you what I've changed).

@mikecote mikecote added Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Mar 8, 2021
@mikecote mikecote self-assigned this Mar 8, 2021
@mikecote mikecote changed the base branch from master to alerting/http-api-terminology March 8, 2021 18:13
@mikecote mikecote marked this pull request as ready for review March 16, 2021 14:20
@mikecote mikecote requested review from a team as code owners March 16, 2021 14:20
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM! As I understood the functional tests will be changed to use the new API when we remove the legacy API support.

@mikecote
Copy link
Contributor Author

As I understood the functional tests will be changed to use the new API when we remove the legacy API support.

They will be done in a follow-up PR to the feature branch. We will have coverage for both sets of APIs by the time the feature branch is merged.

@mikecote mikecote force-pushed the alerting/http-api-terminology branch from 3a69fd8 to 09e8a3b Compare March 16, 2021 18:47
@mikecote mikecote requested a review from ymao1 March 17, 2021 13:51
Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@kibanamachine
Copy link
Contributor

💛 Build succeeded, but was flaky


Test Failures

Kibana Pipeline / general / "before all" hook for "should open a modal".Open timeline Open timeline modal "before all" hook for "should open a modal"

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

AssertionError: Timed out retrying after 60000ms: Expected to find element: `[data-test-subj="title-28bf4e40-8733-11eb-b32c-2515fed320e2"]`, but never found it.

Because this error occurred during a `before all` hook we are skipping the remaining tests in the current suite: `Open timeline`

Although you have test retries enabled, we do not retry tests when `before all` or `after all` hooks fail
    at Object.openTimelineById (http://localhost:6121/__cypress/tests?p=cypress/integration/timelines/open_timeline.spec.ts:16088:15)
    at Context.eval (http://localhost:6121/__cypress/tests?p=cypress/integration/timelines/open_timeline.spec.ts:15046:28)

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
alerting 65.5KB 65.7KB +232.0B

History

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

cc @mikecote

@mikecote mikecote merged commit 4c2a619 into elastic:alerting/http-api-terminology Mar 17, 2021
mikecote added a commit that referenced this pull request Mar 30, 2021
* Move current alert HTTP APIs to legacy folder (#93943)

* Move current HTTP APIs to legacy folder

* Rename BASE_ALERT_API_PATH to LEGACY_BASE_ALERT_API_PATH

* Fix failing tests and extra files

* Create new rule HTTP APIs (#93980)

* Move current HTTP APIs to legacy folder

* Rename BASE_ALERT_API_PATH to LEGACY_BASE_ALERT_API_PATH

* Fix failing tests and extra files

* Move current alert HTTP APIs to legacy folder (#93943)

* Move current HTTP APIs to legacy folder

* Rename BASE_ALERT_API_PATH to LEGACY_BASE_ALERT_API_PATH

* Fix failing tests and extra files

* Add necessary files

* Create rule route

* Get rule API

* Update rule API

* Delete rule route

* Aggregate rules API

* Disable rule API

* Enable rule API

* Find rules API

* Fix Update API

* Get rule alert summary API

* Get rule state API

* Health API

* Rule types API

* Mute all API

* Mute alert API

* Unmute all API

* Unmute alert route

* Update API key API

* corrected tpye by making it much more complicated

* removed unneeded cocde

* Fixes

* Add back health route

* mutedInstanceIds -> mutedAlertIds

* lastRun -> last_run

* alert_type_state -> rule_type_state & alert_instances -> alerts

Co-authored-by: Gidi Meir Morris <[email protected]>

* Create docs for new rule HTTP APIs, deprecate old docs (#94745)

* Create docs for new APIs, deprecate old docs

* Remove connector_type_id

* Update docs

* Add link to legacy APIs from rules API docs

* Remove connector_type_id references

* [DOCS] Add legacy APIs to index.asciidoc

* Fix camel case

Co-authored-by: lcawl <[email protected]>

* Make alerting tests use new rules APIs (#95159)

* Make API integration tests use new HTTP APIs

* Fix end to end tests

* Fix test failures

* Fix more test failures

* Rename some files

* Add tests for legacy APIs (#95333)

* Initial commit (#95457)

* Move some new alerting APIs to /internal (#95461)

* Initial commit

* Update README.md

* Use internal API

* Merge deprecated warning w/ alternative solution

* Update API docs

Co-authored-by: Gidi Meir Morris <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: lcawl <[email protected]>
mikecote added a commit to mikecote/kibana that referenced this pull request Mar 30, 2021
…3977)

* Move current alert HTTP APIs to legacy folder (elastic#93943)

* Move current HTTP APIs to legacy folder

* Rename BASE_ALERT_API_PATH to LEGACY_BASE_ALERT_API_PATH

* Fix failing tests and extra files

* Create new rule HTTP APIs (elastic#93980)

* Move current HTTP APIs to legacy folder

* Rename BASE_ALERT_API_PATH to LEGACY_BASE_ALERT_API_PATH

* Fix failing tests and extra files

* Move current alert HTTP APIs to legacy folder (elastic#93943)

* Move current HTTP APIs to legacy folder

* Rename BASE_ALERT_API_PATH to LEGACY_BASE_ALERT_API_PATH

* Fix failing tests and extra files

* Add necessary files

* Create rule route

* Get rule API

* Update rule API

* Delete rule route

* Aggregate rules API

* Disable rule API

* Enable rule API

* Find rules API

* Fix Update API

* Get rule alert summary API

* Get rule state API

* Health API

* Rule types API

* Mute all API

* Mute alert API

* Unmute all API

* Unmute alert route

* Update API key API

* corrected tpye by making it much more complicated

* removed unneeded cocde

* Fixes

* Add back health route

* mutedInstanceIds -> mutedAlertIds

* lastRun -> last_run

* alert_type_state -> rule_type_state & alert_instances -> alerts

Co-authored-by: Gidi Meir Morris <[email protected]>

* Create docs for new rule HTTP APIs, deprecate old docs (elastic#94745)

* Create docs for new APIs, deprecate old docs

* Remove connector_type_id

* Update docs

* Add link to legacy APIs from rules API docs

* Remove connector_type_id references

* [DOCS] Add legacy APIs to index.asciidoc

* Fix camel case

Co-authored-by: lcawl <[email protected]>

* Make alerting tests use new rules APIs (elastic#95159)

* Make API integration tests use new HTTP APIs

* Fix end to end tests

* Fix test failures

* Fix more test failures

* Rename some files

* Add tests for legacy APIs (elastic#95333)

* Initial commit (elastic#95457)

* Move some new alerting APIs to /internal (elastic#95461)

* Initial commit

* Update README.md

* Use internal API

* Merge deprecated warning w/ alternative solution

* Update API docs

Co-authored-by: Gidi Meir Morris <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: lcawl <[email protected]>
# Conflicts:
#	api_docs/core.json
mikecote added a commit that referenced this pull request Mar 30, 2021
…95781)

* Move current alert HTTP APIs to legacy folder (#93943)

* Move current HTTP APIs to legacy folder

* Rename BASE_ALERT_API_PATH to LEGACY_BASE_ALERT_API_PATH

* Fix failing tests and extra files

* Create new rule HTTP APIs (#93980)

* Move current HTTP APIs to legacy folder

* Rename BASE_ALERT_API_PATH to LEGACY_BASE_ALERT_API_PATH

* Fix failing tests and extra files

* Move current alert HTTP APIs to legacy folder (#93943)

* Move current HTTP APIs to legacy folder

* Rename BASE_ALERT_API_PATH to LEGACY_BASE_ALERT_API_PATH

* Fix failing tests and extra files

* Add necessary files

* Create rule route

* Get rule API

* Update rule API

* Delete rule route

* Aggregate rules API

* Disable rule API

* Enable rule API

* Find rules API

* Fix Update API

* Get rule alert summary API

* Get rule state API

* Health API

* Rule types API

* Mute all API

* Mute alert API

* Unmute all API

* Unmute alert route

* Update API key API

* corrected tpye by making it much more complicated

* removed unneeded cocde

* Fixes

* Add back health route

* mutedInstanceIds -> mutedAlertIds

* lastRun -> last_run

* alert_type_state -> rule_type_state & alert_instances -> alerts

Co-authored-by: Gidi Meir Morris <[email protected]>

* Create docs for new rule HTTP APIs, deprecate old docs (#94745)

* Create docs for new APIs, deprecate old docs

* Remove connector_type_id

* Update docs

* Add link to legacy APIs from rules API docs

* Remove connector_type_id references

* [DOCS] Add legacy APIs to index.asciidoc

* Fix camel case

Co-authored-by: lcawl <[email protected]>

* Make alerting tests use new rules APIs (#95159)

* Make API integration tests use new HTTP APIs

* Fix end to end tests

* Fix test failures

* Fix more test failures

* Rename some files

* Add tests for legacy APIs (#95333)

* Initial commit (#95457)

* Move some new alerting APIs to /internal (#95461)

* Initial commit

* Update README.md

* Use internal API

* Merge deprecated warning w/ alternative solution

* Update API docs

Co-authored-by: Gidi Meir Morris <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: lcawl <[email protected]>
# Conflicts:
#	api_docs/core.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants