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

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented Aug 8, 2019

Part of the new platform migration, task manager should be exposed as a plugin instead of an attribute on the server (server.plugins.task_manager instead of server.taskManager).

This PR converts the task manager to be exposed as a plugin. This also converts task_manager/index.js to typescript to leverage typescript checks to ensure all task manager functions are exposed.

@mikecote mikecote requested a review from a team August 8, 2019 17:50
@mikecote mikecote self-assigned this Aug 8, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-services

@mikecote mikecote requested a review from bmcconaghy August 8, 2019 17:54
@mikecote mikecote added the review label Aug 8, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@bmcconaghy bmcconaghy left a comment

Choose a reason for hiding this comment

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

Noticed some places where we're not handling that the task_manager plugin could be disabled.

@@ -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

@@ -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

Copy link
Contributor

@bmcconaghy bmcconaghy left a comment

Choose a reason for hiding this comment

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

Code LGTM

@mikecote mikecote merged commit f411470 into elastic:master Aug 9, 2019
mikecote added a commit to mikecote/kibana that referenced this pull request Aug 9, 2019
* Expose task manager as plugin instead of server attribute

* Cleanup

* Fix typo
mikecote added a commit that referenced this pull request Aug 9, 2019
…3055)

* Expose task manager as plugin instead of server attribute

* Cleanup

* Fix typo
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 12, 2019
…p-metrics-selectall

* 'master' of github.com:elastic/kibana: (22 commits)
  [Code]: downgrade the log level of error message from subprocess (elastic#42925)
  [Code] Cancel clone/update job in the middle if disk space over the watermark (elastic#42890)
  Add Kibana App specific URL to the help menu (elastic#34739) (elastic#42580)
  [Maps] refactor createShapeFilterWithMeta to support more than just polygons (elastic#43042)
  Skip flaky es_ui_shared/request tests.
  Pass uiSettings to all data plugin services (elastic#42159)
  [SIEM] Upgrades react-redux and utilize React.memo for performance gains  (elastic#43029)
  [skip-ci][Maps] add maki icon sheet to docs (elastic#43063)
  Adding "style-src 'unsafe-inline' 'self'" to default CSP rules (elastic#41305)
  Update dependency commander to v3 (elastic#43041)
  Update dependency @percy/agent to ^0.10.0 (elastic#40517)
  [Maps] only show top hits checkbox if index has date fields (elastic#43056)
  run chained_controls on Firefox to catch regression (elastic#43044)
  fixing issue with dashboard csv download (elastic#42964)
  Expose task manager as plugin instead of server argument (elastic#42966)
  Expose createRouter from HttpService, prepare handlers for context introduction (elastic#42686)
  [Code] disk watermark supports percentage and absolute modes (elastic#42987)
  [apps/dashboard] skip part of filtering tests on FF (elastic#43047)
  [ML] Kibana management jobs list (elastic#42570)
  [ML] Fix check for watcher being enabled (elastic#43025)
  ...
@mikecote mikecote deleted the np/task-manager-as-plugin branch March 18, 2020 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants