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

[ML] Enhances api docs for modules endpoints #66738

Merged
merged 3 commits into from
May 18, 2020

Conversation

peteharverson
Copy link
Contributor

Summary

Enhances the API docs for the ML modules endpoints.

Also contains an edit to the apiDocs script, to fix the processing of the @apiParamExample tag.

@peteharverson peteharverson added review :ml Feature:Anomaly Detection ML anomaly detection v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels May 15, 2020
@peteharverson peteharverson requested a review from a team as a code owner May 15, 2020 14:43
@peteharverson peteharverson self-assigned this May 15, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Comment on lines 89 to 91
* @apiParam {String} indexPatternTitle index pattern to recognize. Note that this does not need to be a Kibana
* index pattern, and can be the name of a single Elasticsearch index,
* or include a wildcard (*) to match multiple indices.
Copy link
Contributor

Choose a reason for hiding this comment

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

please use @apiSchema (params) indexPatternTitleSchema instead

Copy link
Contributor

Choose a reason for hiding this comment

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

It requires the following:

  1. Defined the schema with a unique name in x-pack/plugins/ml/server/routes/schemas/modules.ts like this
export const recognizeIndexParamsSchema = schema.object({
  /**
   * index pattern to recognize. Note that this does not need to be a Kibana
   * index pattern, and can be the name of a single Elasticsearch index,
   * or include a wildcard (*) to match multiple indices.
   */
  indexPatternTitle: schema.string(),
})
  1. import this schema into this file (x-pack/plugins/ml/server/routes/modules.ts) and replace the current inline definition with recognizeIndexParamsSchema.
  2. update annotation to @apiSchema (params) recognizeIndexParamsSchema

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 7a6fd14

Comment on lines 142 to 143
* @apiParam {String} [moduleId] ID of the module to return. If no module ID is supplied,
* an array of all modules will be returned.
Copy link
Contributor

Choose a reason for hiding this comment

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

you can extract

schema.object({
          ...getModuleIdParamSchema(true),
        })

to an export module in x-pack/plugins/ml/server/routes/schemas and use @apiSchema annotation instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 7a6fd14

*
* @apiParam {String} moduleId Module id
* @apiParam {String} moduleId ID of the module to check whether the jobs defined in the module exist.
Copy link
Contributor

Choose a reason for hiding this comment

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

schema.object(getModuleIdParamSchema()) also can be defined as module in schemas

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 7a6fd14

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

@peteharverson
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💛 Build succeeded, but was flaky


Test Failures

Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/uptime/certificates·ts.Uptime app with generated data certificates can navigate to cert page

Link to Jenkins

Standard Out

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

[00:00:00]       │
[00:00:00]         └-: Uptime app
[00:00:00]           └-> "before all" hook
[00:00:00]           └-: with generated data
[00:00:00]             └-> "before all" hook
[00:01:29]             └-: certificates
[00:01:29]               └-> "before all" hook
[00:01:29]               └-> can navigate to cert page
[00:01:29]                 └-> "before each" hook: global before each
[00:01:29]                 └-> "before each" hook: delete settings
[00:01:29]                   │ debg Deleting saved object [object Object]/%s
[00:01:30]                 └-> "before each" hook: load heartbeat data
[00:01:30]                   │ info [uptime/blank] Loading "mappings.json"
[00:01:30]                   │ info [uptime/blank] Loading "data.json"
[00:01:30]                   │ info [o.e.c.m.MetadataCreateIndexService] [kibana-ci-immutable-oraclelinux-tests-xl-1589811470975685913] applying create index request using v1 templates []
[00:01:30]                   │ info [o.e.c.m.MetadataCreateIndexService] [kibana-ci-immutable-oraclelinux-tests-xl-1589811470975685913] [heartbeat-8-generated-test] creating index, cause [api], templates [], shards [1]/[1], mappings [_doc]
[00:01:30]                   │ info [uptime/blank] Created index "heartbeat-8-generated-test"
[00:01:30]                   │ debg [uptime/blank] "heartbeat-8-generated-test" settings undefined
[00:01:30]                 └-> "before each" hook
[00:01:30]                   │ debg TestSubjects.exists(uptimeSettingsToOverviewLink)
[00:01:30]                   │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="uptimeSettingsToOverviewLink"]') with timeout=0
[00:01:30]                   │ debg TestSubjects.click(uptimeSettingsToOverviewLink)
[00:01:30]                   │ debg Find.clickByCssSelector('[data-test-subj="uptimeSettingsToOverviewLink"]') with timeout=10000
[00:01:30]                   │ debg Find.findByCssSelector('[data-test-subj="uptimeSettingsToOverviewLink"]') with timeout=10000
[00:01:30]                   │ debg TestSubjects.exists(uptimeOverviewPage)
[00:01:30]                   │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="uptimeOverviewPage"]') with timeout=2000
[00:01:30]                   │ debg TestSubjects.click(superDatePickerApplyTimeButton)
[00:01:30]                   │ debg Find.clickByCssSelector('[data-test-subj="superDatePickerApplyTimeButton"]') with timeout=10000
[00:01:30]                   │ debg Find.findByCssSelector('[data-test-subj="superDatePickerApplyTimeButton"]') with timeout=10000
[00:01:30]                   │ debg TestSubjects.click(superDatePickerApplyTimeButton)
[00:01:30]                   │ debg Find.clickByCssSelector('[data-test-subj="superDatePickerApplyTimeButton"]') with timeout=10000
[00:01:30]                   │ debg Find.findByCssSelector('[data-test-subj="superDatePickerApplyTimeButton"]') with timeout=10000
[00:01:30]                 │ debg TestSubjects.exists(uptimeCertificatesLink)
[00:01:30]                 │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="uptimeCertificatesLink"]') with timeout=120000
[00:01:31]                 │ debg TestSubjects.click(uptimeCertificatesLink)
[00:01:31]                 │ debg Find.clickByCssSelector('[data-test-subj="uptimeCertificatesLink"]') with timeout=10000
[00:01:31]                 │ debg Find.findByCssSelector('[data-test-subj="uptimeCertificatesLink"]') with timeout=10000
[00:01:31]                 │ debg TestSubjects.exists(uptimeCertificatesPage)
[00:01:31]                 │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="uptimeCertificatesPage"]') with timeout=120000
[00:01:34]                 │ debg --- retry.tryForTime error: [data-test-subj="uptimeCertificatesPage"] is not displayed
[00:01:37]                 │ debg --- retry.tryForTime failed again with the same message...
[00:01:40]                 │ debg --- retry.tryForTime failed again with the same message...
[00:01:43]                 │ debg --- retry.tryForTime failed again with the same message...
[00:01:46]                 │ debg --- retry.tryForTime failed again with the same message...
[00:01:49]                 │ debg --- retry.tryForTime failed again with the same message...
[00:01:52]                 │ debg --- retry.tryForTime failed again with the same message...
[00:01:55]                 │ debg --- retry.tryForTime failed again with the same message...
[00:01:58]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:01]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:04]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:07]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:10]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:13]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:16]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:19]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:22]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:25]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:28]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:31]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:34]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:37]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:40]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:44]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:47]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:50]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:53]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:56]                 │ debg --- retry.tryForTime failed again with the same message...
[00:02:59]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:02]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:05]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:08]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:11]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:14]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:17]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:20]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:23]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:26]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:29]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:32]                 │ debg --- retry.tryForTime failed again with the same message...
[00:03:32]                 │ debg --- retry.try error: expected testSubject(uptimeCertificatesPage) to exist
[00:03:33]                 │ info Taking screenshot "/dev/shm/workspace/kibana/x-pack/test/functional/screenshots/failure/Uptime app with generated data certificates can navigate to cert page.png"
[00:03:33]                 │ info Current URL is: http://localhost:6171/app/uptime#/
[00:03:33]                 │ info Saving page source to: /dev/shm/workspace/kibana/x-pack/test/functional/failure_debug/html/Uptime app with generated data certificates can navigate to cert page.html
[00:03:33]                 └- ✖ fail: "Uptime app with generated data certificates can navigate to cert page"
[00:03:33]                 │

Stack Trace

Error: retry.try timeout: Error: expected testSubject(uptimeCertificatesPage) to exist
    at TestSubjects.existOrFail (/dev/shm/workspace/kibana/test/functional/services/common/test_subjects.ts:62:15)
    at onFailure (/dev/shm/workspace/kibana/test/common/services/retry/retry_for_success.ts:28:9)
    at retryForSuccess (/dev/shm/workspace/kibana/test/common/services/retry/retry_for_success.ts:68:13)

History

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

@peteharverson peteharverson merged commit 1c61ca2 into elastic:master May 18, 2020
@peteharverson peteharverson deleted the ml-modules-api-docs branch May 18, 2020 17:17
peteharverson added a commit to peteharverson/kibana that referenced this pull request May 18, 2020
* [ML] Enhances api docs for modules endpoints

* [ML] Edits to modules schema following review

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
peteharverson added a commit that referenced this pull request May 18, 2020
* [ML] Enhances api docs for modules endpoints

* [ML] Edits to modules schema following review

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 19, 2020
* master: (24 commits)
  [APM] agent config 'profiling_inferred_spans_min_duration' default value is '0ms' but the min value in the field is '1ms' (elastic#66886)
  [Canvas] Fix flaky custom element functional tests (elastic#65908)
  Fix IE specific flexbox min-height issue (elastic#66555)
  [Discover] Unskip doc link functional test (elastic#66884)
  Index pattern management to Kibana platform (elastic#65026)
  Warning and link to support matrix for IE11 (elastic#66512)
  [Reporting] Consolidate Server Type Defs, move some out of Legacy (elastic#66144)
  [SIEM] [Maps] Fixes Network Map empty tooltip (elastic#66828)
  [Endpoint] Encode the index of the alert in the id response (elastic#66919)
  [services/testSubjects] reduce retry usage, add waitForEnabled (elastic#66538)
  [DOCS] Identifies cloud settings for APM (elastic#66935)
  [SIEM][CASE] Fix configuration's page user experience (elastic#66029)
  Resolver: Display node 75% view submenus (elastic#64121)
  [SIEM] Cases] Capture timeline click and open timeline in case view (elastic#66327)
  [APM] Lowercase agent names so icons work (elastic#66824)
  [dev/cli] add support for --no-cache (elastic#66837)
  [Ingest Manager] Better handling of package installation problems (elastic#66541)
  [ML] Enhances api docs for modules endpoints (elastic#66738)
  dont hide errors (elastic#66764)
  [RFC] Global search API (elastic#64284)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Anomaly Detection ML anomaly detection :ml release_note:skip Skip the PR/issue when compiling release notes review v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants