[ML] Switching to new es client #74965
Conversation
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
merge conflict between base and head |
|
retest |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
| await ml.jobServiceProvider(scopedClusterClient, ({} as unknown) as KibanaRequest) | ||
| ).jobsSummary([machineLearningJobId]); | ||
| const fakeRequest = {} as KibanaRequest; | ||
| const summaryJobs = await ml |
There was a problem hiding this comment.
This looks to break ML Rules: when an ML Rule executes, summaryJobs is now undefined, while on master it is MlSummaryJob[].
I apologize for not having tests that exercise this code: we're in the process of moving to the FTR where we can leverage ML setup helpers, but until then we do not have jobs config/anomalies data to test against.
|
@elasticmachine merge upstream |
| .isFullLicense() | ||
| .hasMlCapabilities(['canGetJobs']) | ||
| .ok(async ({ scopedClient }) => { | ||
| const { body } = await scopedClient.asInternalUser.ml.getJobs<{ |
There was a problem hiding this comment.
Is it possible that this logs errors to the Kibana log when querying for a non-existent job, which it didn't before? Is there a less noisy way of checking for a job's existence?
There was a problem hiding this comment.
yes it appears this is a change introduced with using the new js client. Kibana is logging all js client errors to its log.
I am able to disable the log message by passing { ignore: [404] } as an option to getJobs.
I don't know whether this is something we should be doing by default or perhaps exposing this option to consumers of our shared functions.
There was a problem hiding this comment.
For an API that returns a collection I would expect the result set to just not contain the items it couldn't find. 404s/"not found" exceptions mainly make sense for single-resource APIs IMHO.
There was a problem hiding this comment.
Yes, that does make sense, unfortunately this is a basic wrapper around the elasticsearch endpoint, and so behaves in the exact same way.
https://www.elastic.co/guide/en/elasticsearch/reference/master/ml-get-job.html
💚 Build SucceededBuild metricsasync chunks size
distributable file count
History
To update your PR or re-run it, just comment with: |
* [ML] Switching to new es client * further conversions * fixing tests * updating responses * test commit * refactoring shared services to removed context parameter * removing last scoped clients * removing ml js client * udating file data viz errors * fixing jest tests * fixing types after merge with master * error response changes * adding default sizes to some requests * adding custom error types for license checks * tidying up shared function checks * removing test data * removing need for DummyKibanaRequest * updating comment * fixing functional api tests * removing comments * fixing types after master merge * throw error rather than return it * removing placeholder error * changes based on review comments * fixing types after merge with master * fixing missing return Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
* [ML] Switching to new es client * further conversions * fixing tests * updating responses * test commit * refactoring shared services to removed context parameter * removing last scoped clients * removing ml js client * udating file data viz errors * fixing jest tests * fixing types after merge with master * error response changes * adding default sizes to some requests * adding custom error types for license checks * tidying up shared function checks * removing test data * removing need for DummyKibanaRequest * updating comment * fixing functional api tests * removing comments * fixing types after master merge * throw error rather than return it * removing placeholder error * changes based on review comments * fixing types after merge with master * fixing missing return Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
A regression was introduced in elastic#74965 that caused an error to be thrown while collecting telemetry on ML jobs. Because such exceptions are caught and we degrade to zeroing out those counts, this one was not caught until manual testing of telemetry.
A regression was introduced in #74965 that caused an error to be thrown while collecting telemetry on ML jobs. Because such exceptions are caught and we degrade to zeroing out those counts, this one was not caught until manual testing of telemetry.
A regression was introduced in elastic#74965 that caused an error to be thrown while collecting telemetry on ML jobs. Because such exceptions are caught and we degrade to zeroing out those counts, this one was not caught until manual testing of telemetry.
A regression was introduced in #74965 that caused an error to be thrown while collecting telemetry on ML jobs. Because such exceptions are caught and we degrade to zeroing out those counts, this one was not caught until manual testing of telemetry.
A regression was introduced in #74965 that caused an error to be thrown while collecting telemetry on ML jobs. Because such exceptions are caught and we degrade to zeroing out those counts, this one was not caught until manual testing of telemetry.
Changes:
legacyClientbodywhich contains the response payload or error object.requestobject passed to the shared services providers.DummyKibanaRequest. Instead we check that the request is a real instance ofKibanaRequestmessagenow contains less information. So we'll need to dig through the raw error contained in theattributesobject.More information about the new es client can be found here https://github.com/elastic/kibana/blob/master/src/core/MIGRATION_EXAMPLES.md#elasticsearch-client
Checklist
Delete any items that are not applicable to this PR.
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 breaking API changes and was labeled appropriately