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

[TSVB] Shim new platform #39169

Merged
merged 46 commits into from
Aug 5, 2019
Merged

[TSVB] Shim new platform #39169

merged 46 commits into from
Aug 5, 2019

Conversation

gospodarsky
Copy link

@gospodarsky gospodarsky commented Jun 18, 2019

Related to #38615

Summary

  • 1. Identify all public contracts exposed by the existing service (likely not required for most vis type plugins), and create or update the new plugin to re-export the public contracts. This should be a non-breaking change. Plugin should:
    • Be a directory inside of src/legacy/core_plugins
    • Have a new platform-style plugin definition in typescript, with a setup method returning the public contract (see data plugin as an example). This needs to be exported from the top-level /server and/or /public directory, e.g. export foo = new Plugin.setup()
    • Be broken into logical "services", each with a service definition class (see data plugin as an example)
    • Have all static exports done from the top level plugin definition
    • Register the visualization via the "visualizations" plugin's types service.

@gospodarsky gospodarsky added the Feature:TSVB TSVB (Time Series Visual Builder) label Jun 18, 2019
@gospodarsky gospodarsky requested a review from a team June 18, 2019 13:45
@gospodarsky gospodarsky self-assigned this Jun 18, 2019
@gospodarsky gospodarsky requested review from lukeelmers, lizozom, yakhinvadim and streamich and removed request for yakhinvadim June 18, 2019 14:53
@elasticmachine
Copy link
Contributor

💔 Build Failed

@gospodarsky gospodarsky removed the WIP Work in progress label Jun 21, 2019
@gospodarsky gospodarsky changed the title [WIP] [TSVB] Shim new platform [TSVB] Shim new platform Jun 21, 2019
@gospodarsky
Copy link
Author

retest

@gospodarsky
Copy link
Author

@lukeelmers Please, take a look at the changes. I can't be sure that the changes meet your expectations.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

Thanks for getting this started! Added some initial notes, though I'd also like to get feedback from @lizozom or @streamich too.

src/legacy/core_plugins/metrics/index.ts Outdated Show resolved Hide resolved
src/legacy/core_plugins/metrics/public/np/index.ts Outdated Show resolved Hide resolved
src/legacy/core_plugins/metrics/index.ts Outdated Show resolved Hide resolved
src/legacy/core_plugins/metrics/public/np/plugin.ts Outdated Show resolved Hide resolved
src/legacy/core_plugins/metrics/public/np/plugin.ts Outdated Show resolved Hide resolved
src/legacy/core_plugins/metrics/server/np/plugin.ts Outdated Show resolved Hide resolved
src/legacy/core_plugins/metrics/index.ts Outdated Show resolved Hide resolved
@gospodarsky gospodarsky requested a review from alexwizp June 24, 2019 08:04
@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks for your patience during all of this back and forth while we sorted out these first PRs. I just added one minor comment.

My only other feedback would be similar to what I commented on #40032 -- I think we should get rid of setup.ts to avoid confusion and move that shim code directly into legacy.ts (moving forward the plan is to have legacy.ts be the central place for any shim code)

Aside from this, I think we're all done here 😄

src/legacy/core_plugins/metrics/public/plugin.ts Outdated Show resolved Hide resolved
@gospodarsky gospodarsky requested a review from lukeelmers July 15, 2019 10:09
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alexwizp alexwizp self-assigned this Aug 1, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@@ -18,34 +18,39 @@
*/

import { resolve } from 'path';
import { Legacy } from 'kibana';
import { PluginInitializerContext } from 'src/core/server';
import { CoreSetup } from 'src/core/server';
Copy link
Member

Choose a reason for hiding this comment

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

nit: could combine the above two imports

const Private = $injector.get('Private');
const metricsRequestHandler = Private(MetricsRequestHandlerProvider).handler;

async fn(context: Context, args: Arguments) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: context, arguments types don't need to be here

@alexwizp alexwizp merged commit 8d00d26 into elastic:master Aug 5, 2019
alexwizp pushed a commit to alexwizp/kibana that referenced this pull request Aug 5, 2019
* Shim server side to new platform

* Shim public to new platfrom

* Break by services

* Add dependencies to the TSVB plugin

* Change folder structure for the shim

* Pass services as a second argument of setup() and small fixes

* Add start() to the Plugin

* Get rid of the Private

* Pass the core to setup()

* Get rid of NP folder

* Set config to timezoneProvider()

* Take an external dependency from EditorController

* Take an extra dependency out from Request Handler

* Rename metricsPlugin to Plugin

* Fix reviews

* Add types to .setup()

* Change types of TSVB

* Divide the plugin, its setup config and and entry point

* Get rid of @ts-ignore

* Add a server type to the CustomCoreSetup interface

* Revert kbn_vis_type settings

* Restructure public assets

* Move setup.js inner to the legacy.ts

* clean up

* fix PR commnets
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 5, 2019
…-or-edit-existing-rollup-job

* 'master' of github.com:elastic/kibana: (67 commits)
  [TSVB] Shim new platform (elastic#39169)
  [Metric Vis] Shim new platform (elastic#42240)
  [Tag Cloud] Shim new platform (elastic#42348)
  Disable flaky request lib tests. Add es_ui_shared plugin to CODEOWNERS.
  Add disk space percentage to node listing (elastic#42145)
  [SIEM] Add chart interactions - update date picker after brush selection on charts (elastic#42440)
  Document HTTP service (elastic#42331)
  [Reporting] Sanitize 409 error log message (elastic#42495)
  [docs][skip ci] Maps read only access (elastic#35561)
  [x-pack/ftr] refactor types to be more accurate/consistent wit… (elastic#42407)
  [DOCS] Updates images and content in Dashboard docs (elastic#42500)
  Allow sorting on multiple columns in Discover (elastic#41918)
  [Infra UI][Logs UI] Fix autocomplete to use proper derived index pattern (elastic#42287)
  [ftr/cheerio] improve cheerio types to include test subject me… (elastic#42534)
  Upgraded EUI 13.0.0 -> 13.1.1 (elastic#42298)
  Increase max-old-space-size for builds (elastic#42218)
  [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint (elastic#41836)
  [Logs UI][a11y] Announce name of column on remove column button (elastic#41695)
  Inspector 👉 New Platform (elastic#42164)
  Make alerting properly space aware (elastic#42081)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 5, 2019
…s_autocomplete

* 'master' of github.com:elastic/kibana: (189 commits)
  [TSVB] Shim new platform (elastic#39169)
  [Metric Vis] Shim new platform (elastic#42240)
  [Tag Cloud] Shim new platform (elastic#42348)
  Disable flaky request lib tests. Add es_ui_shared plugin to CODEOWNERS.
  Add disk space percentage to node listing (elastic#42145)
  [SIEM] Add chart interactions - update date picker after brush selection on charts (elastic#42440)
  Document HTTP service (elastic#42331)
  [Reporting] Sanitize 409 error log message (elastic#42495)
  [docs][skip ci] Maps read only access (elastic#35561)
  [x-pack/ftr] refactor types to be more accurate/consistent wit… (elastic#42407)
  [DOCS] Updates images and content in Dashboard docs (elastic#42500)
  Allow sorting on multiple columns in Discover (elastic#41918)
  [Infra UI][Logs UI] Fix autocomplete to use proper derived index pattern (elastic#42287)
  [ftr/cheerio] improve cheerio types to include test subject me… (elastic#42534)
  Upgraded EUI 13.0.0 -> 13.1.1 (elastic#42298)
  Increase max-old-space-size for builds (elastic#42218)
  [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint (elastic#41836)
  [Logs UI][a11y] Announce name of column on remove column button (elastic#41695)
  Inspector 👉 New Platform (elastic#42164)
  Make alerting properly space aware (elastic#42081)
  ...
alexwizp added a commit that referenced this pull request Aug 5, 2019
* Shim server side to new platform

* Shim public to new platfrom

* Break by services

* Add dependencies to the TSVB plugin

* Change folder structure for the shim

* Pass services as a second argument of setup() and small fixes

* Add start() to the Plugin

* Get rid of the Private

* Pass the core to setup()

* Get rid of NP folder

* Set config to timezoneProvider()

* Take an external dependency from EditorController

* Take an extra dependency out from Request Handler

* Rename metricsPlugin to Plugin

* Fix reviews

* Add types to .setup()

* Change types of TSVB

* Divide the plugin, its setup config and and entry point

* Get rid of @ts-ignore

* Add a server type to the CustomCoreSetup interface

* Revert kbn_vis_type settings

* Restructure public assets

* Move setup.js inner to the legacy.ts

* clean up

* fix PR commnets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration Feature:TSVB TSVB (Time Series Visual Builder) release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants