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

Expose task manager as plugin instead of server argument #42966

Merged
merged 3 commits into from
Aug 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/actions/server/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function init(server: Legacy.Server) {
};
}

const { taskManager } = server;
const taskManager = server.plugins.task_manager!;
const actionTypeRegistry = new ActionTypeRegistry({
getServices,
taskManager: taskManager!,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/alerting/server/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function init(server: Legacy.Server) {
};
}

const { taskManager } = server;
const taskManager = server.plugins.task_manager!;
const alertTypeRegistry = new AlertTypeRegistry({
getServices,
taskManager: taskManager!,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { TASK_ID, scheduleTask, registerMapsTelemetryTask } from './telemetry_ta

export function initTelemetryCollection(server) {
registerMapsTelemetryTask(server);
scheduleTask(server, server.taskManager);
scheduleTask(server, server.plugins.task_manager);
bmcconaghy marked this conversation as resolved.
Show resolved Hide resolved
registerMapsUsageCollector(server);
}

Expand All @@ -20,8 +20,9 @@ async function isTaskManagerReady(server) {

async function fetch(server) {
let docs;
const taskManager = server.plugins.task_manager;
try {
({ docs } = await server.taskManager.fetch({
({ docs } = await taskManager.fetch({
query: {
bool: {
filter: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function scheduleTask(server, taskManager) {
}

export function registerMapsTelemetryTask(server) {
const taskManager = server.taskManager;
const taskManager = server.plugins.task_manager;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, should check for null.

Copy link
Member

Choose a reason for hiding this comment

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

Just checking in on the changes, which LGTM

A null check for this might be moot to do from the maps plugin, since the plugin definition requires task_manager: 55e7b183d75#diff-1bb2390b8f55e83f3a830c385a7d664eR21

It's odd that part ^^ worked before this PR

taskManager.registerTaskDefinitions({
[TELEMETRY_TASK_TYPE]: {
title: 'Maps telemetry fetch task',
Expand Down
10 changes: 5 additions & 5 deletions x-pack/legacy/plugins/maps/server/test_utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,18 @@ export const getMockTaskFetch = (docs = defaultMockTaskDocs) => {
export const getMockKbnServer = (
mockCallWithInternal = getMockCallWithInternal(),
mockTaskFetch = getMockTaskFetch()) => ({
taskManager: {
registerTaskDefinitions: () => undefined,
schedule: () => Promise.resolve(),
fetch: mockTaskFetch,
},
plugins: {
elasticsearch: {
getCluster: () => ({
callWithInternalUser: mockCallWithInternal,
}),
},
xpack_main: {},
task_manager: {
registerTaskDefinitions: () => undefined,
schedule: () => Promise.resolve(),
fetch: mockTaskFetch,
},
},
usage: {
collectorSet: {
Expand Down
18 changes: 9 additions & 9 deletions x-pack/legacy/plugins/oss_telemetry/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,6 @@ export interface TaskInstance {
}

export interface HapiServer {
taskManager: {
registerTaskDefinitions: (opts: any) => void;
schedule: (opts: any) => Promise<void>;
fetch: (
opts: any
) => Promise<{
docs: TaskInstance[];
}>;
};
plugins: {
xpack_main: any;
elasticsearch: {
Expand All @@ -53,6 +44,15 @@ export interface HapiServer {
callWithInternalUser: () => Promise<ESQueryResponse>;
};
};
task_manager: {
registerTaskDefinitions: (opts: any) => void;
schedule: (opts: any) => Promise<void>;
fetch: (
opts: any
) => Promise<{
docs: TaskInstance[];
}>;
};
};
usage: {
collectorSet: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ async function isTaskManagerReady(server: HapiServer) {
}

async function fetch(server: HapiServer) {
const { taskManager } = server;
const taskManager = server.plugins.task_manager!;

let docs;
try {
Expand Down
4 changes: 2 additions & 2 deletions x-pack/legacy/plugins/oss_telemetry/server/lib/tasks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { PLUGIN_ID, VIS_TELEMETRY_TASK, VIS_TELEMETRY_TASK_NUM_WORKERS } from '.
import { visualizationsTaskRunner } from './visualizations/task_runner';

export function registerTasks(server: HapiServer) {
const { taskManager } = server;
const taskManager = server.plugins.task_manager;
Copy link
Contributor

Choose a reason for hiding this comment

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

same here re: null check


taskManager.registerTaskDefinitions({
[VIS_TELEMETRY_TASK]: {
Expand All @@ -26,7 +26,7 @@ export function registerTasks(server: HapiServer) {
}

export function scheduleTasks(server: HapiServer) {
const { taskManager } = server;
const taskManager = server.plugins.task_manager;
const { kbnServer } = server.plugins.xpack_main.status.plugin;

kbnServer.afterPluginsInit(() => {
Expand Down
10 changes: 5 additions & 5 deletions x-pack/legacy/plugins/oss_telemetry/test_utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,18 @@ export const getMockKbnServer = (
mockCallWithInternal = getMockCallWithInternal(),
mockTaskFetch = getMockTaskFetch()
): HapiServer => ({
taskManager: {
registerTaskDefinitions: (opts: any) => undefined,
schedule: (opts: any) => Promise.resolve(),
fetch: mockTaskFetch,
},
plugins: {
elasticsearch: {
getCluster: (cluster: string) => ({
callWithInternalUser: mockCallWithInternal,
}),
},
xpack_main: {},
task_manager: {
registerTaskDefinitions: (opts: any) => undefined,
schedule: (opts: any) => Promise.resolve(),
fetch: mockTaskFetch,
},
},
usage: {
collectorSet: {
Expand Down
8 changes: 4 additions & 4 deletions x-pack/legacy/plugins/task_manager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ The task_manager can be configured via `taskManager` config options (e.g. `taskM

## Task definitions

Plugins define tasks by calling the `registerTaskDefinitions` method on the `server.taskManager` object.
Plugins define tasks by calling the `registerTaskDefinitions` method on the `server.plugins.task_manager` object.

A sample task can be found in the [x-pack/test/plugin_api_integration/plugins/task_manager](../../test/plugin_api_integration/plugins/task_manager/index.js) folder.

```js
const { taskManager } = server;
const taskManager = server.plugins.task_manager;
taskManager.registerTaskDefinitions({
// clusterMonitoring is the task type, and must be unique across the entire system
clusterMonitoring: {
Expand Down Expand Up @@ -215,7 +215,7 @@ The data stored for a task instance looks something like this:
The task manager mixin exposes a taskManager object on the Kibana server which plugins can use to manage scheduled tasks. Each method takes an optional `scope` argument and ensures that only tasks with the specified scope(s) will be affected.

```js
const { taskManager } = server;
const taskManager = server.plugins.task_manager;
// Schedules a task. All properties are as documented in the previous
// storage section, except that here, params is an object, not a JSON
// string.
Expand Down Expand Up @@ -258,7 +258,7 @@ For example:

```js
// In your plugin's init
server.taskManager.addMiddleware({
server.plugins.task_manager.addMiddleware({
async beforeSave({ taskInstance, ...opts }) {
console.log(`About to save a task of type ${taskInstance.taskType}`);

Expand Down
8 changes: 0 additions & 8 deletions x-pack/legacy/plugins/task_manager/index.d.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,29 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Root } from 'joi';
import { Legacy } from 'kibana';
import { SavedObjectsSerializer, SavedObjectsSchema } from '../../../../src/core/server';
import { TaskManager } from './task_manager';
import { TaskManager as TaskManagerClass } from './task_manager';
import mappings from './mappings.json';
import { migrations } from './migrations';
import { TaskManager } from './types';

export function taskManager(kibana) {
export { TaskManager };
export { TaskInstance, ConcreteTaskInstance, TaskRunCreatorFunction } from './task';

export function taskManager(kibana: any) {
return new kibana.Plugin({
id: 'task_manager',
require: ['kibana', 'elasticsearch', 'xpack_main'],
configPrefix: 'xpack.task_manager',
config(Joi) {
config(Joi: Root) {
return Joi.object({
enabled: Joi.boolean().default(true),
max_attempts: Joi.number()
.description('The maximum number of times a task will be attempted before being abandoned as failed')
.description(
'The maximum number of times a task will be attempted before being abandoned as failed'
)
.min(1)
.default(3),
poll_interval: Joi.number()
Expand All @@ -29,16 +37,20 @@ export function taskManager(kibana) {
.description('The name of the index used to store task information.')
.default('.kibana_task_manager'),
max_workers: Joi.number()
.description('The maximum number of tasks that this Kibana instance will run simultaneously.')
.description(
'The maximum number of tasks that this Kibana instance will run simultaneously.'
)
.min(1) // disable the task manager rather than trying to specify it with 0 workers
.default(10),
override_num_workers: Joi.object()
.pattern(/.*/, Joi.number().greater(0))
.description('Customize the number of workers occupied by specific tasks (e.g. override_num_workers.reporting: 2)')
.default({})
.description(
'Customize the number of workers occupied by specific tasks (e.g. override_num_workers.reporting: 2)'
)
.default({}),
}).default();
},
init(server) {
init(server: Legacy.Server) {
const config = server.config();
const schema = new SavedObjectsSchema(this.kbnServer.uiExports.savedObjectSchemas);
const serializer = new SavedObjectsSerializer(schema);
Expand All @@ -48,13 +60,20 @@ export function taskManager(kibana) {
['task']
);

const taskManager = new TaskManager({
const taskManagerInstance = new TaskManagerClass({
kbnServer: this.kbnServer,
config,
savedObjectsRepository,
serializer,
});
server.decorate('server', 'taskManager', taskManager);
const exposedFunctions: TaskManager = {
fetch: (...args) => taskManagerInstance.fetch(...args),
remove: (...args) => taskManagerInstance.remove(...args),
schedule: (...args) => taskManagerInstance.schedule(...args),
addMiddleware: (...args) => taskManagerInstance.addMiddleware(...args),
registerTaskDefinitions: (...args) => taskManagerInstance.registerTaskDefinitions(...args),
};
server.expose(exposedFunctions);
},
uiExports: {
mappings,
Expand All @@ -64,7 +83,7 @@ export function taskManager(kibana) {
hidden: true,
isNamespaceAgnostic: true,
convertToAliasScript: `ctx._id = ctx._source.type + ':' + ctx._id`,
indexPattern(config) {
indexPattern(config: any) {
return config.get('xpack.task_manager.index');
},
},
Expand Down
6 changes: 2 additions & 4 deletions x-pack/legacy/plugins/task_manager/task_manager.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { TaskManager } from './task_manager';

type Schema = PublicMethodsOf<TaskManager>;
import { TaskManager } from './types';

const createTaskManagerMock = () => {
const mocked: jest.Mocked<Schema> = {
const mocked: jest.Mocked<TaskManager> = {
registerTaskDefinitions: jest.fn(),
addMiddleware: jest.fn(),
schedule: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default function (kibana) {
},

init(server) {
const { taskManager } = server;
const taskManager = server.plugins.task_manager;

taskManager.registerTaskDefinitions({
sampleTask: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const taskManagerQuery = {
};

export function initRoutes(server) {
const { taskManager } = server;
const taskManager = server.plugins.task_manager;

server.route({
path: '/api/sample_tasks',
Expand Down
4 changes: 1 addition & 3 deletions x-pack/test/typings/hapi.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ import { TaskManager } from '../../legacy/plugins/task_manager';
import { AlertingPlugin, AlertsClient } from '../../legacy/plugins/alerting';

declare module 'hapi' {
interface Server {
taskManager?: TaskManager;
}
interface Request {
getActionsClient?: () => ActionsClient;
getAlertsClient?: () => AlertsClient;
Expand All @@ -29,5 +26,6 @@ declare module 'hapi' {
encrypted_saved_objects?: EncryptedSavedObjectsPlugin;
actions?: ActionsPlugin;
alerting?: AlertingPlugin;
task_manager?: TaskManager;
}
}
4 changes: 1 addition & 3 deletions x-pack/typings/hapi.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ import { TaskManager } from '../legacy/plugins/task_manager';
import { AlertingPlugin, AlertsClient } from '../legacy/plugins/alerting';

declare module 'hapi' {
interface Server {
taskManager?: TaskManager;
}
interface Request {
getActionsClient?: () => ActionsClient;
getAlertsClient?: () => AlertsClient;
Expand All @@ -29,5 +26,6 @@ declare module 'hapi' {
encrypted_saved_objects?: EncryptedSavedObjectsPlugin;
actions?: ActionsPlugin;
alerting?: AlertingPlugin;
task_manager?: TaskManager;
}
}