[ML] New Platform server shim: update anomaly detectors routes to use new platform router #56717
Conversation
alvarezmelissa87
added
:ml
v8.0.0
release_note:skip
|
Pinging @elastic/ml-ui (:ml) |
alvarezmelissa87
force-pushed
the
ml-new-platform-job-routes
branch
from
7b08fed to
f713ea6
Compare
alvarezmelissa87
requested review from
peteharverson,
jgowdyelastic and
darnautov
x-pack/legacy/plugins/ml/server/new_platform/anomaly_detectors_schema.ts
Outdated
Show resolved
Hide resolved
x-pack/legacy/plugins/ml/server/new_platform/anomaly_detectors_schema.ts
Outdated
Show resolved
Hide resolved
| { | ||
| path: '/api/ml/anomaly_detectors/_validate/detector', | ||
| validate: { | ||
| body: schema.any(), |
There was a problem hiding this comment.
seems like body schema validation is missing 🙂
There was a problem hiding this comment.
I don't see this one being used anywhere 🤔 so I held off adding a schema as it would make it pretty restrictive.
| * | ||
| * @api {put} /api/ml/anomaly_detectors/:jobId/_update Update an anomaly detection job | ||
| * @apiName UpdateAnomalyDetectors | ||
| * @apiDescription This API updates an anomaly detection job |
There was a problem hiding this comment.
Copying from the ML docs, Updates certain properties of an anomaly detection job.
There was a problem hiding this comment.
Updated in b70059756cf4d0eb6a125b0ba7e891c69daa622d
| */ | ||
| router.get( | ||
| { | ||
| path: '/api/ml/anomaly_detectors/{jobId}/results/categories/{categoryId}', |
There was a problem hiding this comment.
I don't think anywhere in the UI is actually calling this route, but no harm in leaving it in, as a way of accessing the categories info via the ML endpoint.
There was a problem hiding this comment.
Thanks for taking a look and for the confirmation 😄 I agree, sounds good to leave it in. 👌
alvarezmelissa87
force-pushed
the
ml-new-platform-job-routes
branch
from
57ffe5e to
b700597
Compare
There was a problem hiding this comment.
Can you add entries to the ml/server/routes/apidoc.json for the apiGroup and apiNames of each route, to ensure they get included in the docs generated by the tool. Use the same order as the routes are defined in the file:
"AnomalyDetectors",
"GetAnomalyDetectors",
"GetAnomalyDetectorsById",
"GetAnomalyDetectorsStatsById"
...
alvarezmelissa87
force-pushed
the
ml-new-platform-job-routes
branch
from
1e8e4a6 to
5601920
Compare
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
… new platform router (elastic#56717) * update job routes to NP * call notify.error properly on mlMessageBarService * add route documentation annotation to job routes * update schema and route doc * update apidoc file with routes
Summary
Related meta issue: #49743
Validation audit: https://github.com/elastic/kibana-team/issues/167
Updates all
anomaly_detectorsroutes to use new platform router.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] This was checked for cross-browser compatibility, including a check against IE11- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support- [ ] Unit or functional tests were updated or added to match the most common scenarios- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
- [ ] This was checked for breaking API changes and was labeled appropriately- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately