Skip to content

Use platform types vs our own. Hook up NP via kbnServer.newPlatform.#39824

Merged
jfsiii merged 1 commit intoelastic:feature-integrations-managerfrom
jfsiii:use-kbnserver-and-more-np
Jun 28, 2019
Merged

Use platform types vs our own. Hook up NP via kbnServer.newPlatform.#39824
jfsiii merged 1 commit intoelastic:feature-integrations-managerfrom
jfsiii:use-kbnserver-and-more-np

Conversation

@jfsiii
Copy link
Copy Markdown
Contributor

@jfsiii jfsiii commented Jun 27, 2019

Summary

Use KbnServer so we get NP features in kbnServer.newPlatform.

Currently using for calling the plugin setup with the NP CoreStart of kbnServer.newPlatform.setup.core

@jfsiii jfsiii requested review from a team, jasonrhodes and rudolf June 27, 2019 18:31
Copy link
Copy Markdown
Contributor

@rudolf rudolf left a comment

Choose a reason for hiding this comment

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

You could switch to only use CoreSetup and CoreStart from src/core/server. At some point you might require functionality that isn't provided in Core but will eventually be moved there (like SavedObjects) so then you could create an type like interface IntegrationsCoreSetup extends CoreSetup {...} to add these dependencies internal to your plugin until they become available.

import { fetchList } from './registry';
import { routes } from './routes';

export interface CoreSetup {
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.

You can also pull in CoreSetup from src/core/server

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.

I tried that at one point but got this error
Screen Shot 2019-06-28 at 9 28 00 AM

I can avoid it using InternalCoreSetup but that didn’t seem better.
Screen Shot 2019-06-28 at 9 28 58 AM

I took a pass at importing from KbnServer or src/legacy but didn’t get anywhere. Happy to learn about other options.

// `kbnServer.newPlatform` has important values
const kbnServer = (server as unknown) as KbnServer;
const initializerContext: PluginInitializerContext = {};
const coreSetup: CoreSetup = kbnServer.newPlatform.setup.core;
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.

👍 now you're using the actual new platform instead of approximating it with a shim.

constructor(initializerContext: PluginInitializerContext) {}
public setup(core: CoreSetup) {
const { route } = core.http;
const { server } = core.http;
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.

http.server is there for backwards compatibility/legacy, but you can also switch to http.registerRouter to only rely on new platform functionality.

Copy link
Copy Markdown
Contributor

@mshustov mshustov Jun 28, 2019

Choose a reason for hiding this comment

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

just a reminder that route handler interfaces is not stabilized yet. it will be exposed once we implement #39767

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.

@skaapgif lol, you’re calling out all my side quests from #39833. There was a version of it with that. I didn’t use it because it felt too early (@restrry’s point about the rfc, issues w/import, etc). Happy to try it out anytime.

Copy link
Copy Markdown
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

lgtm! exciting to see this new plugin stuff shape up

@jfsiii jfsiii merged commit 7b5d64b into elastic:feature-integrations-manager Jun 28, 2019
@jfsiii jfsiii deleted the use-kbnserver-and-more-np branch June 28, 2019 19:51
@jen-huang jen-huang added the Team:Fleet Team label for Observability Data Collection Fleet team label Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Fleet Team label for Observability Data Collection Fleet team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants