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

[ML] New Platform server shim: update results service routes to use new platform router #56886

Merged
merged 5 commits into from
Feb 6, 2020
Merged

[ML] New Platform server shim: update results service routes to use new platform router #56886

merged 5 commits into from
Feb 6, 2020

Conversation

alvarezmelissa87
Copy link
Contributor

Summary

Related meta issue: #49743
Validation audit: https://github.com/elastic/kibana-team/issues/167

Updates all results_service routes to use new platform router.
Updates results_service model to ts file. It should still be able to be used by legacy code still using callWithRequest

Checklist

Delete any items that are not applicable to this PR.

@alvarezmelissa87 alvarezmelissa87 requested a review from a team as a code owner February 5, 2020 17:24
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@alvarezmelissa87 alvarezmelissa87 self-assigned this Feb 5, 2020
@alvarezmelissa87 alvarezmelissa87 force-pushed the ml-new-platform-results-service-routes branch from 0e2d2c8 to ddb9d9f Compare February 5, 2020 21:05
peteharverson
peteharverson reviewed Feb 6, 2020
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

A few minor text edits for the API docs. You also need to add the apiGroup and apiNames to the apidoc.json file to ensure they get added to the docs.

x-pack/legacy/plugins/ml/server/routes/results_service.ts Outdated
*
* @api {post} /api/ml/results/max_anomaly_score Returns the maximum anomaly_score
* @apiName GetMaxAnomalyScore
* @apiDescription Returns the maximum anomaly_score for result_type:bucket over jobIds for the interval passed in
Copy link
Contributor

Choose a reason for hiding this comment

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

Edit to Returns the maximum anomaly score of the bucket results for the request job ID(s) and time range.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 0574d51

x-pack/legacy/plugins/ml/server/routes/results_service.ts Outdated
/**
* @apiGroup ResultsService
*
* @api {post} /api/ml/results/category_examples Returns the maximum anomaly_score
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy / paste error - should be Returns category examples

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 0574d51

x-pack/legacy/plugins/ml/server/routes/results_service.ts Outdated
*
* @api {post} /api/ml/results/category_examples Returns the maximum anomaly_score
* @apiName GetCategoryExamples
* @apiDescription Returns the categorization examples for the categories with the specified IDs from the given index and job ID
Copy link
Contributor

Choose a reason for hiding this comment

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

Edit to Returns examples for the categories with the specified IDs from the job with the supplied ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 0574d51

x-pack/legacy/plugins/ml/server/routes/results_service.ts Outdated
/**
* @apiGroup ResultsService
*
* @api {post} /api/ml/results/partition_fields_values Returns partition fields
Copy link
Contributor

Choose a reason for hiding this comment

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

Edit to Returns partition fields values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 0574d51

x-pack/legacy/plugins/ml/server/routes/results_service.ts Outdated
*
* @api {post} /api/ml/results/partition_fields_values Returns partition fields
* @apiName GetPartitionFieldsValues
* @apiDescription Returns the record of partition fields with possible values that fit the provided queries.
Copy link
Contributor

Choose a reason for hiding this comment

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

Returns the partition fields with values that match the provided criteria for the specified job ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 0574d51

darnautov
darnautov reviewed Feb 6, 2020
View reviewed changes
x-pack/legacy/plugins/ml/server/routes/results_service.ts
{
path: '/api/ml/results/anomalies_table_data',
validate: {
body: schema.object({ ...anomaliesTableDataSchema }),
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need a shallow copy here?

Copy link
Contributor

Choose a reason for hiding this comment

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

and all the other places with validate 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.

I saw this pattern initially when I was looking for examples 🤔 Though you're right, we don't really need it. Happy to change these if it makes sense. 😄

darnautov
darnautov approved these changes Feb 6, 2020
View reviewed changes
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

@alvarezmelissa87 alvarezmelissa87 force-pushed the ml-new-platform-results-service-routes branch from ddb9d9f to 20d3ea8 Compare February 6, 2020 15:27
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

  • 💔 Build #24739 failed ddb9d9f5cbe4ab88e5b5e207eb3a6bd8e766e048
  • 💚 Build #24657 succeeded 0e2d2c822a985eed9e2f83ce1d540ab6ddcda393

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

peteharverson
peteharverson approved these changes Feb 6, 2020
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Latest edits LGTM

@alvarezmelissa87 alvarezmelissa87 merged commit 3d9b461 into elastic:master Feb 6, 2020
@alvarezmelissa87 alvarezmelissa87 mentioned this pull request Feb 6, 2020
Merged
alvarezmelissa87 added a commit to alvarezmelissa87/kibana that referenced this pull request Feb 6, 2020
@alvarezmelissa87
[ML] New Platform server shim: update results service routes to use n…
ff85c7d 
…ew platform router (elastic#56886)

* migrate resultsService routes to NP. begin conversion of model file to TS

* add schema validation to routes

* add types to results service model file

* add docs for routes

* update route description and add routes to doc json file
@alvarezmelissa87 alvarezmelissa87 deleted the ml-new-platform-results-service-routes branch February 6, 2020 20:59
@peteharverson peteharverson mentioned this pull request Feb 6, 2020
Closed
78 tasks
alvarezmelissa87 added a commit that referenced this pull request Feb 6, 2020
@alvarezmelissa87
[ML] New Platform server shim: update results service routes to use n…
e8b4fa6 
…ew platform router (#56886) (#57040)

* migrate resultsService routes to NP. begin conversion of model file to TS

* add schema validation to routes

* add types to results service model file

* add docs for routes

* update route description and add routes to doc json file
jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 7, 2020
@jloleysens
Merge branch 'console/feature/text-objects-in-saved-objects' of githu…
9e266c6 
…b.com:jloleysens/kibana into console/feature/text-objects-in-saved-objects

* 'console/feature/text-objects-in-saved-objects' of github.com:jloleysens/kibana: (103 commits)
  fix auto closing new vis modal when navigating to lens or when navigating away with browser history (elastic#56998)
  TS of esKuery\node_types  (elastic#56857)
  Kibana app migration: Move static code dependencies into kibana_legacy plugin, part 1 (elastic#56408)
  Retry ES API calls that fail with 410/Gone (elastic#56950)
  [APM] Show missing permissions message to the user on the Services overview (elastic#56374)
  Fixing flaky CI tests for custom appRoutes (elastic#55763)
  [State Management][Docs] State syncing utils docs (elastic#56479)
  [Index management] Remove index mapper setting in tests (elastic#57066)
  Exposed common EuiExpressions to separate components be able to reuse for building new for Alert Types  (elastic#56466)
  [SIEM] update url state between page if date is relative (elastic#56813)
  fix for chart_types test (elastic#57056)
  chore(NA): remove compress from dll minimizer (elastic#57023)
  [File upload] Migrate routing to NP & add route validation (elastic#52313)
  Adding docs for grouped nav advanced setting (elastic#57013)
  Use i18n titles for field formatters, human names for numeral locales (elastic#56348)
  [Maps] Remove EMS catalogue url from docs (elastic#57020)
  [Endpoint] ERT-82 ERT-83 ERT-84: Alert list API with pagination (elastic#56538)
  [DOCS] Adds Apple notarization info to install doc (elastic#57042)
  [ML] New Platform server shim: update results service routes to use new platform router (elastic#56886)
  Fix typo on detection engine rule (elastic#56993)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peteharverson peteharverson peteharverson approved these changes

@darnautov darnautov darnautov approved these changes

@jgowdyelastic jgowdyelastic Awaiting requested review from jgowdyelastic

Assignees

@alvarezmelissa87 alvarezmelissa87

Labels
Feature:NP Migration :ml release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@alvarezmelissa87 @elasticmachine @kibanamachine @darnautov @peteharverson