[ML] Removes duplicated service code used in Single Metric Viewer #178566

alvarezmelissa87 · 2024-03-12T20:10:09Z

Summary

Related meta issue: #176651

This PR removes duplicates created in the initial PR for adding SMV as an embeddable in dashboards:

ml/public/application/services/forecast_service_provider.ts replaces ml/public/application/services/forecast_service.js
ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/time_series_search_service.ts replaces ml/public/application/timeseriesexplorer/timeseries_search_service.ts
removes ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/timeseriesexplorer_utils.js

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
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

...l/public/application/timeseriesexplorer/timeseriesexplorer_utils/timeseriesexplorer_utils.js

walterra

Overall nice reduction of duplicated code! 🥳 Just added some naming suggestions and one about withKibana/context. To play it safe, maybe let's do a PR first with functional tests of the Single Metric Viewer embedding and then get this one in afterwards?

x-pack/plugins/ml/public/application/services/forecast_service_provider.ts

...s/ml/public/application/timeseriesexplorer/components/forecasting_modal/forecasting_modal.js

peteharverson

Overall looks good. Manual testing of the chart inside ML and Dashboard as an embeddable all working as expected.

alvarezmelissa87 · 2024-03-14T20:08:30Z

Overall nice reduction of duplicated code! 🥳 Just added some naming suggestions and one about withKibana/context. To play it safe, maybe let's do a PR first with functional tests of the Single Metric Viewer embedding and then get this one in afterwards?

Thanks for taking a look @walterra - added functional tests here: #178768

alvarezmelissa87 · 2024-03-14T21:23:46Z

Updated with suggested changes. Also removed timeseriesexplorer_utils.js and removed the need for the final function in it createTimeSeriesJobData.

Ready for another test/look when you get a chance cc @peteharverson, @walterra
I will hold off on merging until the functional test PR is merged and this PR is updated with latest from main. 👍

alvarezmelissa87 · 2024-03-18T19:14:58Z

@elasticmachine merge upstream

elasticmachine · 2024-03-18T19:15:17Z

Pinging @elastic/ml-ui (:ml)

walterra

Latest changes LGTM! 🚀

peteharverson

Latest changes LGTM

jgowdyelastic

Added some small comments, but on the whole LGTM.

jgowdyelastic · 2024-03-19T11:29:20Z

...s/ml/public/application/timeseriesexplorer/components/forecasting_modal/forecasting_modal.js

@@ -546,4 +557,4 @@ export class ForecastingModalUI extends Component {
  }
 }

-export const ForecastingModal = withKibana(ForecastingModalUI);
+export const ForecastingModal = ForecastingModalUI;


ForecastingModalUI isn't needed anymore now. The class can just called ForecastingModal

Updated in 70a4278

jgowdyelastic · 2024-03-19T11:32:30Z

.../public/application/jobs/jobs_list/components/job_details/forecasts_table/forecasts_table.js

@@ -21,12 +21,12 @@ import {

 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n-react';
-import { withKibana } from '@kbn/kibana-react-plugin/public';
+import { withKibana, context } from '@kbn/kibana-react-plugin/public';


Looks like the same can be done here, we can remove withKibana as we now use context.

Updated in 70a4278

jgowdyelastic · 2024-03-19T11:38:31Z

x-pack/plugins/ml/public/application/util/time_series_explorer_service.ts

+  expression: string;
+}
+
+export interface ChartDataPoint {


Nit, these don't need to be exported anymore as they are only used inside this file.

Updated in 70a4278

alvarezmelissa87 · 2024-03-19T18:51:42Z

@elasticmachine merge upstream

alvarezmelissa87 · 2024-03-19T21:51:41Z

/ci

alvarezmelissa87 · 2024-03-19T22:32:49Z

@elasticmachine merge upstream

kibana-ci · 2024-03-19T23:44:22Z

💚 Build Succeeded

Buildkite Build
Commit: 67fdafd

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1950	1946	-4

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`ml`	3.7MB	3.7MB	-8.7KB

History

💔 Build #198373 failed 749bbc9
💔 Build #198348 failed 70a4278

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

cc @alvarezmelissa87

alvarezmelissa87 added 2 commits March 12, 2024 13:22

remove duplicates for forecast service

5ceb5e3

replace old timeseriesSearchService

6783635

alvarezmelissa87 added :ml Feature:Anomaly Detection ML anomaly detection release_note:skip Skip the PR/issue when compiling release notes v8.14.0 labels Mar 12, 2024

alvarezmelissa87 self-assigned this Mar 12, 2024

alvarezmelissa87 requested review from peteharverson and jgowdyelastic March 12, 2024 20:10

alvarezmelissa87 commented Mar 12, 2024

View reviewed changes

...l/public/application/timeseriesexplorer/timeseriesexplorer_utils/timeseriesexplorer_utils.js Outdated Show resolved Hide resolved

alvarezmelissa87 mentioned this pull request Mar 12, 2024

[ML] Anomaly Detection: Single Metric Viewer embeddable - 8.14 enhancements #176651

Closed

13 tasks

alvarezmelissa87 requested a review from walterra March 13, 2024 13:56

update jest tests

cc17d0c

walterra reviewed Mar 14, 2024

View reviewed changes

peteharverson reviewed Mar 14, 2024

View reviewed changes

alvarezmelissa87 added 2 commits March 14, 2024 14:25

rename forecast service

e33fb0d

remove unused utils file

e4d02f9

Merge branch 'main' into ml-smv-remove-service-duplicates

1af3d17

alvarezmelissa87 marked this pull request as ready for review March 18, 2024 19:15

alvarezmelissa87 requested a review from a team as a code owner March 18, 2024 19:15

alvarezmelissa87 mentioned this pull request Mar 19, 2024

[ML] Consolidate redundant time_buckets into @kbn/ml-time-buckets. #178756

Merged

2 tasks

walterra approved these changes Mar 19, 2024

View reviewed changes

peteharverson approved these changes Mar 19, 2024

View reviewed changes

jgowdyelastic approved these changes Mar 19, 2024

View reviewed changes

remove unnecessary withKibana

70a4278

Merge branch 'main' into ml-smv-remove-service-duplicates

749bbc9

Merge branch 'main' into ml-smv-remove-service-duplicates

67fdafd

walterra merged commit 8c85463 into elastic:main Mar 20, 2024
16 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Mar 20, 2024

alvarezmelissa87 deleted the ml-smv-remove-service-duplicates branch March 20, 2024 14:37

walterra mentioned this pull request Mar 20, 2024

[ML][Meta] Technical debt and maintenance work for 8.14.0 #178782

Closed

14 tasks

peteharverson mentioned this pull request Apr 10, 2024

[ML] Migrate Javascript files to Typescript #153894

Open

39 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Removes duplicated service code used in Single Metric Viewer #178566

[ML] Removes duplicated service code used in Single Metric Viewer #178566

alvarezmelissa87 commented Mar 12, 2024 •

edited

Loading

walterra left a comment

peteharverson left a comment

alvarezmelissa87 commented Mar 14, 2024

alvarezmelissa87 commented Mar 14, 2024

alvarezmelissa87 commented Mar 18, 2024

elasticmachine commented Mar 18, 2024

walterra left a comment

peteharverson left a comment

jgowdyelastic left a comment

jgowdyelastic Mar 19, 2024

alvarezmelissa87 Mar 19, 2024

jgowdyelastic Mar 19, 2024

alvarezmelissa87 Mar 19, 2024

jgowdyelastic Mar 19, 2024

alvarezmelissa87 Mar 19, 2024

alvarezmelissa87 commented Mar 19, 2024

alvarezmelissa87 commented Mar 19, 2024

alvarezmelissa87 commented Mar 19, 2024

kibana-ci commented Mar 19, 2024

[ML] Removes duplicated service code used in Single Metric Viewer #178566

[ML] Removes duplicated service code used in Single Metric Viewer #178566

Conversation

alvarezmelissa87 commented Mar 12, 2024 • edited Loading

Summary

Checklist

walterra left a comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

alvarezmelissa87 commented Mar 14, 2024

alvarezmelissa87 commented Mar 14, 2024

alvarezmelissa87 commented Mar 18, 2024

elasticmachine commented Mar 18, 2024

walterra left a comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

jgowdyelastic left a comment

Choose a reason for hiding this comment

jgowdyelastic Mar 19, 2024

Choose a reason for hiding this comment

alvarezmelissa87 Mar 19, 2024

Choose a reason for hiding this comment

jgowdyelastic Mar 19, 2024

Choose a reason for hiding this comment

alvarezmelissa87 Mar 19, 2024

Choose a reason for hiding this comment

jgowdyelastic Mar 19, 2024

Choose a reason for hiding this comment

alvarezmelissa87 Mar 19, 2024

Choose a reason for hiding this comment

alvarezmelissa87 commented Mar 19, 2024

alvarezmelissa87 commented Mar 19, 2024

alvarezmelissa87 commented Mar 19, 2024

kibana-ci commented Mar 19, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

alvarezmelissa87 commented Mar 12, 2024 •

edited

Loading