Skip to content

Initial framework for data plugin. - #34350

Merged
lukeelmers merged 4 commits into
elastic:masterfrom
lukeelmers:poc/data-plugin
Apr 15, 2019
Merged

lukeelmers merged 4 commits into
elastic:masterfrom
lukeelmers:poc/data-plugin

Conversation

@lukeelmers

@lukeelmers lukeelmers commented Apr 2, 2019 •

Copy link
Copy Markdown
Contributor

We've had some conversations around the idea of creating a new platform plugin that is owned by @elastic/kibana-app-arch and houses any services that apps might rely on for retrieving & managing data in kibana:

  • interpreter & expressions
  • courier & querying infrastructure
  • index patterns
  • filter bar, query bar, time picker (and possibly the corresponding UIs too, TBD)

At the same time, we are trying to sort out the best path forward for consolidating items from ui/public into their respective locations.

The idea here is pretty straightforward:

  1. We create a legacy plugin in the shape of the data plugin mentioned above
  2. This plugin will have services created for each item we think it should own
  3. For now, services will simply re-export items from their existing locations (whether they be in plugins/ or ui/public)
  4. Downstream imports can be updated across kibana to point to the data plugin
  5. Once that's complete, we can then actually move items into the plugin without breaking anything

Here's an example of what importing would look like:

// in the new platform, the below import will be removed as `data` will already be
// getting passed in to the service
import * as data from 'plugins/data';

// using functions
const { IndexPatternsProvider } = data.indexPatterns;
const indexPatterns = IndexPatternsProvider(whatever);

// using constants
const { INDEX_PATTERN_ILLEGAL_CHARACTERS } = data.indexPatterns.constants;
console.log(`Using constants: ${INDEX_PATTERN_ILLEGAL_CHARACTERS}`);

// using ui componenets
const { IndexPatternSelect } = data.indexPatterns.ui;
<IndexPatternSelect />

This has the benefit of giving the service the general shape of the new platform, where everything will be injected in the plugin's setup() method as data.

For unstable items, or things we know will have an API changed in the near future, we could consider adding a legacy namespace so you would import like:

import * as data from 'plugins/data';

data.indexPatterns.legacy.someLegacyFunction();

And then move items out of legacy when we sort out a longer-term API.

Still TBD:

  • Conventions for exporting types
    • Core has established some conventions for exporting each service's Setup types, which I think we should mirror over time and I've tried to mimic here
    • For miscellaneous types that already exist and need to be exported, I'm explicitly adding those to the returned value of setup within each service for now, inside a types object
  • Handling for server stuff (this is so far focused on client stuff since we're trying to use it as a means of eliminating ui/public)
    • Since there's not too much server stuff we need to add initially, I won't worry too much about this yet.
  • What to do with testing utilities like mocks & fixtures.
    • With app arch plugins specifically, I think we should model this after core and provide first-class mocks for each service, since app developers will be building on top of our services

@lukeelmers lukeelmers Apr 4, 2019 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The shape of what's returned here is completely arbitrary and something we'll need to design on a per-service basis, though I do think we can work to establish some conventions over time.

Like in this case, I've put a React component that index patterns already exports under ui, and also grouped fields, constants, fixtures... but this could really be designed however we feel is best.

I'm also not sure how we want to deal with exporting one-off types that aren't part of the Setup interface... currently they are nested under types

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

right now nothing will use this so i am ok with merging it as it is. we will explore the actual interface as part of deangularization pr (in which we will also update all the consumers of index pattern service to import from this new place)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, that makes sense to me. For some future services where we end up exporting stuff like this as an interim step, the way it is structured will become more important. But in the case of index patterns, this was mostly meant to be illustrative.

@lukeelmers lukeelmers removed the WIP Work in progress label Apr 4, 2019
@lukeelmers
lukeelmers marked this pull request as ready for review April 4, 2019 15:46
@lukeelmers

Copy link
Copy Markdown
Contributor Author

I think this is ready for review & an initial merge now so that we can continue adding more services to it.

@ppisljar @lizozom I've updated the index patterns service to export what I think are all of the current public contracts, but of course those will keep evolving as we finish de-angularizing in #34418.

cc @timroes @epixa @stacey-gammon

@lukeelmers
lukeelmers requested review from epixa, lizozom and ppisljar April 4, 2019 15:50
@lukeelmers lukeelmers changed the title POC for data plugin that re-exports existing services. Initial framework for data plugin. Apr 4, 2019
@elasticmachine

This comment has been minimized.

@lukeelmers

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@ppisljar ppisljar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

}

/** @public */
export type IndexPatternsSetup = ReturnType<IndexPatternsService['setup']>;

@lukeelmers lukeelmers Apr 8, 2019 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks like the approach to exporting types in Core is changing in #34725. We might consider revisiting this part when we actually move index patterns over so that we can keep things consistent.

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@lukeelmers
lukeelmers merged commit 61a4b04 into elastic:master Apr 15, 2019
@lukeelmers
lukeelmers deleted the poc/data-plugin branch April 15, 2019 21:41
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Apr 15, 2019
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
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.

3 participants