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] Adding job sychronization warning to job list pages #85794

2 changes: 1 addition & 1 deletion x-pack/plugins/ml/common/types/saved_objects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export type JobsSpacesResponse = {
};

export interface InitializeSavedObjectResponse {
jobs: Array<{ id: string; type: string }>;
jobs: Array<{ id: string; type: JobType }>;
success: boolean;
error?: any;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export { SavedObjectsWarning } from './saved_objects_warning';
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC, useEffect, useState } from 'react';

import { EuiCallOut, EuiLink, EuiSpacer } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { JobType } from '../../../../common/types/saved_objects';
import { useMlApiContext, useMlKibana } from '../../contexts/kibana';

interface Props {
jobType?: JobType;
}

export const SavedObjectsWarning: FC<Props> = ({ jobType }) => {
const {
savedObjects: { initSavedObjects },
} = useMlApiContext();
const {
services: {
http: { basePath },
},
} = useMlKibana();

const [showWarning, setShowWarning] = useState(false);

useEffect(() => {
let unmounted = false;
initSavedObjects(true)
.then(({ jobs }) => {
if (unmounted === true) {
return;
}

const missingJobs =
jobs.length > 0 && (jobType === undefined || jobs.some(({ type }) => type === jobType));
setShowWarning(missingJobs);
Copy link
Contributor

Choose a reason for hiding this comment

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

this effect potentially can update the state of an unmounted component

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't this the case for all async calls that are made from a useEffect?
Normally we call an async function but do not await on it as you can't pass an async function to useEffect, here I decided to use a then rather than create a second function.
This is a pattern i've seen in plenty of places, is there a better alternative?

Copy link
Contributor

Choose a reason for hiding this comment

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

Check out this example

, I don't know if there is a better way

Copy link
Member Author

Choose a reason for hiding this comment

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

ok yeah, a lock like this is a good solution.
We have a lot of components which have this potential problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated in 4eedc54

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, you're right, I see a warning message from React on this issue quite often.

})
.catch(() => {
console.log('Saved object synchronization check could not be performed.'); // eslint-disable-line no-console
});
return () => {
unmounted = true;
};
}, []);

return showWarning === false ? null : (
<>
<EuiCallOut
title={
<FormattedMessage
id="xpack.ml.jobsList.missingSavedObjectWarning.title"
defaultMessage="ML job synchronization needed"
/>
}
color="warning"
iconType="alert"
>
<div>
<FormattedMessage
id="xpack.ml.jobsList.missingSavedObjectWarning.description"
defaultMessage="All jobs require an accompanying saved object. Some jobs are missing their saved object and require synchronization in the {link}."
values={{
link: (
<EuiLink href={`${basePath.get()}/app/management/insightsAndAlerting/jobsListLink`}>
<FormattedMessage
id="xpack.ml.jobsList.missingSavedObjectWarning.linkToManagement.link"
defaultMessage="stack management page"
/>
</EuiLink>
),
}}
/>
</div>
</EuiCallOut>
<EuiSpacer size="m" />
</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { DataFrameAnalyticsList } from './components/analytics_list';
import { useRefreshInterval } from './components/analytics_list/use_refresh_interval';
import { RefreshAnalyticsListButton } from './components/refresh_analytics_list_button';
import { NodeAvailableWarning } from '../../../components/node_available_warning';
import { SavedObjectsWarning } from '../../../components/saved_objects_warning';
import { UpgradeWarning } from '../../../components/upgrade';
import { AnalyticsNavigationBar } from './components/analytics_navigation_bar';
import { ModelsList } from './components/models_management';
Expand Down Expand Up @@ -106,6 +107,7 @@ export const Page: FC = () => {
</EuiPageHeader>

<NodeAvailableWarning />
<SavedObjectsWarning jobType="data-frame-analytics" />
<UpgradeWarning />

<EuiPageContent>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { MultiJobActions } from '../multi_job_actions';
import { NewJobButton } from '../new_job_button';
import { JobStatsBar } from '../jobs_stats_bar';
import { NodeAvailableWarning } from '../../../../components/node_available_warning';
import { SavedObjectsWarning } from '../../../../components/saved_objects_warning';
import { DatePickerWrapper } from '../../../../components/navigation_menu/date_picker_wrapper';
import { UpgradeWarning } from '../../../../components/upgrade';
import { RefreshJobsListButton } from '../refresh_jobs_list_button';
Expand Down Expand Up @@ -439,6 +440,7 @@ export class JobsListView extends Component {
</EuiPageHeader>

<NodeAvailableWarning />
<SavedObjectsWarning jobType="anomaly-detector" />

<UpgradeWarning />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { NavigationMenu } from '../components/navigation_menu';
import { OverviewSideBar } from './components/sidebar';
import { OverviewContent } from './components/content';
import { NodeAvailableWarning } from '../components/node_available_warning';
import { SavedObjectsWarning } from '../components/saved_objects_warning';
import { UpgradeWarning } from '../components/upgrade';

export const OverviewPage: FC = () => {
Expand All @@ -26,6 +27,7 @@ export const OverviewPage: FC = () => {
<EuiPage data-test-subj="mlPageOverview">
<EuiPageBody>
<NodeAvailableWarning />
<SavedObjectsWarning />
<UpgradeWarning />

<EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { basePath } from './index';
import {
JobType,
SyncSavedObjectResponse,
InitializeSavedObjectResponse,
SavedObjectResult,
JobsSpacesResponse,
} from '../../../../common/types/saved_objects';
Expand Down Expand Up @@ -47,4 +48,12 @@ export const savedObjectsApiProvider = (httpService: HttpService) => ({
query: { simulate },
});
},

initSavedObjects(simulate: boolean = false) {
return httpService.http<InitializeSavedObjectResponse>({
path: `${basePath()}/saved_objects/initialize`,
method: 'GET',
query: { simulate },
});
},
});