Skip to content

Commit

Permalink
[ML] Job service refactor (#191724)
Browse files Browse the repository at this point in the history
Follow on from dependancy cache removal
#189729

Moving and removing as much as possible out of the `JobService` class.

- Moving the job cloning functions to a separate class as they do not
require the same dependancies as `JobService`
- Removing api basic wrapper functions like `saveNewJob`, `openJob`,
`forceStartDatafeeds` etc
- Removes toast dependency. This might be controversial, but I think it
is unnecessary for these basic job/datafeed loading functions to show a
toast if they fail. If we cannot load the basic list of jobs then there
is something else very wrong with the system. We also throw the errors,
so they will not be lost. It should be up to the calling function to
display a toast. Removing this dependency cleans up the code quite a
lot.
- The `JobCreator` classes no longer use `JobService`
  • Loading branch information
jgowdyelastic authored Sep 10, 2024
1 parent 08f70b7 commit 2d40fa3
Show file tree
Hide file tree
Showing 60 changed files with 417 additions and 708 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import { withKibana } from '@kbn/kibana-react-plugin/public';

import { addItemToRecentlyAccessed } from '../../../util/recently_accessed';
import { mlJobServiceFactory } from '../../../services/job_service';
import { toastNotificationServiceProvider } from '../../../services/toast_notification_service';
import { mlTableService } from '../../../services/table_service';
import { ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE } from '../../../../../common/constants/search';
import {
Expand Down Expand Up @@ -101,10 +100,7 @@ class AnnotationsTableUI extends Component {
this.sorting = {
sort: { field: 'timestamp', direction: 'asc' },
};
this.mlJobService = mlJobServiceFactory(
toastNotificationServiceProvider(props.kibana.services.notifications.toasts),
props.kibana.services.mlServices.mlApi
);
this.mlJobService = mlJobServiceFactory(props.kibana.services.mlServices.mlApi);
}

getAnnotations() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ export const LinksMenuUI = (props: LinksMenuProps) => {
if (dataView === null) {
return;
}

dataView.getIndexPattern();
const field = findMessageField(dataView);
if (field !== null) {
setMessageField(field);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,20 @@ import {
ANOMALY_DETECTION_DEFAULT_TIME_RANGE,
ANOMALY_DETECTION_ENABLE_TIME_RANGE,
} from '../../../../common/constants/settings';
import { useMlJobService } from '../../services/job_service';
import { createResultsUrlForJobs } from '../../util/results_url';

export const useCreateADLinks = () => {
const {
services: {
http: { basePath },
},
} = useMlKibana();
const mlJobService = useMlJobService();

const useUserTimeSettings = useUiSettings().get(ANOMALY_DETECTION_ENABLE_TIME_RANGE);
const userTimeSettings = useUiSettings().get(ANOMALY_DETECTION_DEFAULT_TIME_RANGE);
const createLinkWithUserDefaults = useCallback(
(location: any, jobList: any) => {
const resultsUrl = mlJobService.createResultsUrlForJobs(
(location, jobList) => {
const resultsUrl = createResultsUrlForJobs(
jobList,
location,
useUserTimeSettings === true && userTimeSettings !== undefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import type { DataFrameAnalyticsConfig } from '@kbn/ml-data-frame-analytics-utils';

import { createDatafeedId } from '../../../../../common/util/job_utils';
import type { JobType } from '../../../../../common/types/saved_objects';
import type { Job, Datafeed } from '../../../../../common/types/anomaly_detection_jobs';
import type { Filter } from '../../../../../common/types/filters';
Expand Down Expand Up @@ -105,7 +106,7 @@ export class JobImportService {
const { jobId } = jobIds[i];
j.job.job_id = jobId;
j.datafeed.job_id = jobId;
j.datafeed.datafeed_id = `datafeed-${jobId}`;
j.datafeed.datafeed_id = createDatafeedId(jobId);
return j;
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,7 @@ class RuleEditorFlyoutUI extends Component {
this.partitioningFieldNames = [];
this.canGetFilters = checkPermission('canGetFilters');

this.mlJobService = mlJobServiceFactory(
toastNotificationServiceProvider(props.kibana.services.notifications.toasts),
props.kibana.services.mlServices.mlApi
);
this.mlJobService = mlJobServiceFactory(props.kibana.services.mlServices.mlApi);
}

componentDidMount() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ import {
EuiButton,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { useMlKibana } from '../../../../contexts/kibana';
import { useMlApi, useMlKibana } from '../../../../contexts/kibana';
import type { MlSummaryJob } from '../../../../../../common/types/anomaly_detection_jobs';
import { isManagedJob } from '../../../jobs_utils';
import { useMlJobService } from '../../../../services/job_service';
import { closeJobs } from '../utils';
import { ManagedJobsWarningCallout } from './managed_jobs_warning_callout';

Expand All @@ -44,7 +43,7 @@ export const CloseJobsConfirmModal: FC<Props> = ({
notifications: { toasts },
},
} = useMlKibana();
const mlJobService = useMlJobService();
const mlApi = useMlApi();
const [modalVisible, setModalVisible] = useState(false);
const [hasManagedJob, setHasManaged] = useState(true);
const [jobsToReset, setJobsToReset] = useState<MlSummaryJob[]>([]);
Expand Down Expand Up @@ -121,7 +120,7 @@ export const CloseJobsConfirmModal: FC<Props> = ({

<EuiButton
onClick={() => {
closeJobs(toasts, mlJobService, jobsToReset, refreshJobs);
closeJobs(toasts, mlApi, jobsToReset, refreshJobs);
closeModal();
}}
fill
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ import {
EuiButton,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { useMlKibana } from '../../../../contexts/kibana';
import { useMlApi, useMlKibana } from '../../../../contexts/kibana';
import type { MlSummaryJob } from '../../../../../../common/types/anomaly_detection_jobs';
import { isManagedJob } from '../../../jobs_utils';
import { useMlJobService } from '../../../../services/job_service';
import { stopDatafeeds } from '../utils';
import { ManagedJobsWarningCallout } from './managed_jobs_warning_callout';

Expand All @@ -45,7 +44,7 @@ export const StopDatafeedsConfirmModal: FC<Props> = ({
notifications: { toasts },
},
} = useMlKibana();
const mlJobService = useMlJobService();
const mlApi = useMlApi();
const [modalVisible, setModalVisible] = useState(false);
const [hasManagedJob, setHasManaged] = useState(true);
const [jobsToStop, setJobsToStop] = useState<MlSummaryJob[]>([]);
Expand Down Expand Up @@ -122,7 +121,7 @@ export const StopDatafeedsConfirmModal: FC<Props> = ({

<EuiButton
onClick={() => {
stopDatafeeds(toasts, mlJobService, jobsToStop, refreshJobs);
stopDatafeeds(toasts, mlApi, jobsToStop, refreshJobs);
closeModal();
}}
fill
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ import {
} from '@elastic/eui';

import { i18n } from '@kbn/i18n';
import { useMlKibana } from '../../../../contexts/kibana';
import { useMlApi, useMlKibana } from '../../../../contexts/kibana';
import { deleteJobs } from '../utils';
import { BLOCKED_JOBS_REFRESH_INTERVAL_MS } from '../../../../../../common/constants/jobs_list';
import { DeleteSpaceAwareItemCheckModal } from '../../../../components/delete_space_aware_item_check_modal';
import { useMlJobService } from '../../../../services/job_service';
import type { MlSummaryJob } from '../../../../../../common/types/anomaly_detection_jobs';
import { isManagedJob } from '../../../jobs_utils';
import { ManagedJobsWarningCallout } from '../confirm_modals/managed_jobs_warning_callout';
Expand All @@ -46,7 +45,7 @@ export const DeleteJobModal: FC<Props> = ({ setShowFunction, unsetShowFunction,
notifications: { toasts },
},
} = useMlKibana();
const mlJobService = useMlJobService();
const mlApi = useMlApi();
const [deleting, setDeleting] = useState(false);
const [modalVisible, setModalVisible] = useState(false);
const [adJobs, setAdJobs] = useState<MlSummaryJob[]>([]);
Expand Down Expand Up @@ -92,7 +91,7 @@ export const DeleteJobModal: FC<Props> = ({ setShowFunction, unsetShowFunction,
setDeleting(true);
deleteJobs(
toasts,
mlJobService,
mlApi,
jobIds.map((id) => ({ id })),
deleteUserAnnotations,
deleteAlertingRules
Expand All @@ -102,9 +101,7 @@ export const DeleteJobModal: FC<Props> = ({ setShowFunction, unsetShowFunction,
closeModal();
refreshJobs();
}, BLOCKED_JOBS_REFRESH_INTERVAL_MS);
// exclude mlJobservice from deps
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [jobIds, deleteUserAnnotations, deleteAlertingRules, closeModal, refreshJobs]);
}, [toasts, mlApi, jobIds, deleteUserAnnotations, deleteAlertingRules, closeModal, refreshJobs]);

if (modalVisible === false || jobIds.length === 0) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import { EuiFieldText, EuiForm, EuiFormRow, EuiSpacer, EuiTitle } from '@elastic
import { FormattedMessage } from '@kbn/i18n-react';
import { context } from '@kbn/kibana-react-plugin/public';

import { mlJobServiceFactory } from '../../../../../services/job_service';
import { toastNotificationServiceProvider } from '../../../../../services/toast_notification_service';
import { detectorToString } from '../../../../../util/string_utils';

export class Detectors extends Component {
Expand All @@ -23,13 +21,6 @@ export class Detectors extends Component {
constructor(props, constructorContext) {
super(props, constructorContext);

const mlJobService = mlJobServiceFactory(
toastNotificationServiceProvider(constructorContext.services.notifications.toasts),
constructorContext.services.mlServices.mlApi
);

this.detectors = mlJobService.getJobGroups().map((g) => ({ label: g.id }));

this.state = {
detectors: [],
detectorDescriptions: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export function actionsMenuContent(
toastNotifications,
application,
mlApi,
mlJobService,
showEditJobFlyout,
showDatafeedChartFlyout,
showDeleteJobModal,
Expand Down Expand Up @@ -77,7 +76,7 @@ export function actionsMenuContent(
if (isManagedJob(item)) {
showStopDatafeedsConfirmModal([item]);
} else {
stopDatafeeds(toastNotifications, mlJobService, [item], refreshJobs);
stopDatafeeds(toastNotifications, mlApi, [item], refreshJobs);
}

closeMenu(true);
Expand Down Expand Up @@ -114,7 +113,7 @@ export function actionsMenuContent(
if (isManagedJob(item)) {
showCloseJobsConfirmModal([item]);
} else {
closeJobs(toastNotifications, mlJobService, [item], refreshJobs);
closeJobs(toastNotifications, mlApi, [item], refreshJobs);
}

closeMenu(true);
Expand Down Expand Up @@ -153,7 +152,7 @@ export function actionsMenuContent(
return isJobBlocked(item) === false && canCreateJob;
},
onClick: (item) => {
cloneJob(toastNotifications, application, mlApi, mlJobService, item.id);
cloneJob(toastNotifications, application, mlApi, item.id);
closeMenu(true);
},
'data-test-subj': 'mlActionButtonCloneJob',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import { AnomalyDetectionJobIdLink } from './job_id_link';
import { isManagedJob } from '../../../jobs_utils';
import { mlJobServiceFactory } from '../../../../services/job_service';
import { toastNotificationServiceProvider } from '../../../../services/toast_notification_service';

const PAGE_SIZE_OPTIONS = [10, 25, 50];

Expand All @@ -47,10 +45,6 @@ export class JobsListUI extends Component {
};

this.mlApi = props.kibana.services.mlServices.mlApi;
this.mlJobService = mlJobServiceFactory(
toastNotificationServiceProvider(props.kibana.services.notifications.toasts),
this.mlApi
);
}

static getDerivedStateFromProps(props) {
Expand Down Expand Up @@ -341,7 +335,6 @@ export class JobsListUI extends Component {
this.props.kibana.services.notifications.toasts,
this.props.kibana.services.application,
this.mlApi,
this.mlJobService,
this.props.showEditJobFlyout,
this.props.showDatafeedChartFlyout,
this.props.showDeleteJobModal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';

import { withKibana } from '@kbn/kibana-react-plugin/public';

import { checkForAutoStartDatafeed, filterJobs, loadFullJob } from '../utils';
import { filterJobs, loadFullJob } from '../utils';
import { JobsList } from '../jobs_list';
import { JobDetails } from '../job_details';
import { JobFilterBar } from '../job_filter_bar';
Expand All @@ -37,8 +37,7 @@ import { StopDatafeedsConfirmModal } from '../confirm_modals/stop_datafeeds_conf
import { CloseJobsConfirmModal } from '../confirm_modals/close_jobs_confirm_modal';
import { AnomalyDetectionEmptyState } from '../anomaly_detection_empty_state';
import { removeNodeInfo } from '../../../../../../common/util/job_utils';
import { mlJobServiceFactory } from '../../../../services/job_service';
import { toastNotificationServiceProvider } from '../../../../services/toast_notification_service';
import { jobCloningService } from '../../../../services/job_cloning_service';

let blockingJobsRefreshTimeout = null;

Expand Down Expand Up @@ -80,11 +79,6 @@ export class JobsListViewUI extends Component {
* @private
*/
this._isFiltersSet = false;

this.mlJobService = mlJobServiceFactory(
toastNotificationServiceProvider(props.kibana.services.notifications.toasts),
props.kibana.services.mlServices.mlApi
);
}

componentDidMount() {
Expand All @@ -106,7 +100,7 @@ export class JobsListViewUI extends Component {
}

openAutoStartDatafeedModal() {
const job = checkForAutoStartDatafeed(this.mlJobService);
const job = jobCloningService.checkForAutoStartDatafeed();
if (job !== undefined) {
this.showStartDatafeedModal([job]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import { context } from '@kbn/kibana-react-plugin/public';

import { checkPermission } from '../../../../capabilities/check_capabilities';
import { mlNodesAvailable } from '../../../../ml_nodes_check/check_ml_nodes';
import { mlJobServiceFactory } from '../../../../services/job_service';
import { toastNotificationServiceProvider } from '../../../../services/toast_notification_service';

import { isManagedJob } from '../../../jobs_utils';

Expand Down Expand Up @@ -46,12 +44,8 @@ class MultiJobActionsMenuUI extends Component {
this.canResetJob = checkPermission('canResetJob') && mlNodesAvailable();
this.canCreateMlAlerts = checkPermission('canCreateMlAlerts');

this.toastNoticiations = constructorContext.services.notifications.toasts;
const mlApi = constructorContext.services.mlServices.mlApi;
const toastNotificationService = toastNotificationServiceProvider(
constructorContext.services.notifications.toasts
);
this.mlJobService = mlJobServiceFactory(toastNotificationService, mlApi);
this.toastNotifications = constructorContext.services.notifications.toasts;
this.mlApi = constructorContext.services.mlServices.mlApi;
}

onButtonClick = () => {
Expand Down Expand Up @@ -116,7 +110,7 @@ class MultiJobActionsMenuUI extends Component {
if (this.props.jobs.some((j) => isManagedJob(j))) {
this.props.showCloseJobsConfirmModal(this.props.jobs);
} else {
closeJobs(this.toastNotifications, this.mlJobService, this.props.jobs);
closeJobs(this.toastNotifications, this.mlApi, this.props.jobs);
}

this.closePopover();
Expand Down Expand Up @@ -163,7 +157,12 @@ class MultiJobActionsMenuUI extends Component {
if (this.props.jobs.some((j) => isManagedJob(j))) {
this.props.showStopDatafeedsConfirmModal(this.props.jobs);
} else {
stopDatafeeds(this.props.jobs, this.props.refreshJobs);
stopDatafeeds(
this.toastNotifications,
this.mlApi,
this.props.jobs,
this.props.refreshJobs
);
}
this.closePopover();
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ export class GroupSelectorUI extends Component {
}

const tempJobs = newJobs.map((j) => ({ jobId: j.id, groups: j.newGroups }));
const ml = this.props.kibana.services.mlServices.mlApi;
ml.jobs
const mlApi = this.props.kibana.services.mlServices.mlApi;
mlApi.jobs
.updateGroups(tempJobs)
.then((resp) => {
let success = true;
Expand Down
Loading

0 comments on commit 2d40fa3

Please sign in to comment.