[Code] migrate Code config/logging/feature service to new platform by mw-ding · Pull Request #46664 · elastic/kibana

mw-ding · 2019-09-26T00:23:52Z

Summary

Finish the config, logging and feature service items in https://github.com/elastic/code/issues/1596

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-09-26T00:23:54Z

Pinging @elastic/code

elasticmachine · 2019-09-26T01:42:24Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-27T20:22:42Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-27T22:29:40Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-28T00:48:59Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

mshustov · 2019-09-30T14:26:14Z

x-pack/plugins/code/server/config.ts

optional: as a common convention we don't import legacy platform in the new platform, but we encourage to do the other way around. can we move some definitions to the new platform plugin and import them from legacy platform?

added a TODO here. These paths anyway will break when moving all the plugins here. The error will enforce the update or the path.

x-pack/plugins/code/server/config.ts

mshustov · 2019-09-30T14:48:05Z

x-pack/plugins/code/server/config.ts

right now it breaks you typings due to any https://github.com/elastic/kibana/pull/46664/files#diff-1b9e65e1fa500e8a7255ba665e6b1922R17

see my previous comment regarding this issue.

x-pack/legacy/plugins/code/index.ts

x-pack/plugins/code/server/plugin.ts

x-pack/legacy/plugins/code/server/utils/server_logger_factory.ts

mshustov · 2019-09-30T15:03:27Z

x-pack/legacy/plugins/code/server/plugin.ts

I see the problem that we have to pass LoggerFactory down the tree. Hopefully, we can improve this in #39695

x-pack/legacy/plugins/code/server/log.ts

mw-ding

@restrry another question here is how to access the shared config like path.data

https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/code/server/server_options.ts#L41

mw-ding · 2019-09-30T18:50:14Z

x-pack/plugins/code/server/config.ts

added a TODO here. These paths anyway will break when moving all the plugins here. The error will enforce the update or the path.

x-pack/plugins/code/server/config.ts

x-pack/legacy/plugins/code/index.ts

x-pack/legacy/plugins/code/server/log.ts

x-pack/legacy/plugins/code/server/utils/server_logger_factory.ts

x-pack/plugins/code/server/plugin.ts

mw-ding · 2019-09-30T22:25:14Z

@restrry another question here is how to access the shared config like path.data

https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/code/server/server_options.ts#L41

chatted with @joshdover offline that for the shared config item like path.data, I will leave them as they are right now and migrate the way to access the config later.

elasticmachine · 2019-09-30T23:50:16Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

mw-ding · 2019-10-01T01:23:20Z

x-pack/legacy/plugins/code/index.ts

I leave the enabled flag in both here and the config in the new place with default value true. However, the code app is disabled by default without puting xpack.code.enabled in the kibana.yml. Any idea on how to fix this? @restrry @joshdover

Can we use? enabled: Joi.boolean().default(false)

why do we want to have false by default?

the code app is disabled by default

because it's disabled by default.

So this is how it works now. We keep Code enabled on master and 7.x, but disabled on the release branch. Even in the release branch, we set the xpack.code.ui.enabled to false to disable, but still keep xpack.code.enabled to be true.

We keep Code enabled on master and 7.x, but disabled on the release branch

how do you achieve that? changing default values in legacy platform config?

elasticmachine · 2019-10-01T02:29:37Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

mshustov · 2019-10-02T08:41:58Z

x-pack/legacy/plugins/code/index.ts

You might want to add a comment to revisit after #41990
The issue already has a high priority

this comment is intended for the same purpose

x-pack/legacy/plugins/code/server/plugin.ts

mshustov · 2019-10-02T09:15:31Z

x-pack/legacy/plugins/code/server/lsp/language_servers.ts

LspOptions typings are not full. there is no info about available languages and typings are broken due to https://github.com/elastic/kibana/pull/46664/files#r329619654

This is a legacy issue for the LspOptions specifically. I will defer to @spacedragon to fix this later. In short, I think let's focus more on the major NP service migration in this PR.

x-pack/legacy/plugins/code/server/utils/console_logger.ts

x-pack/plugins/code/server/plugin.ts

elasticmachine · 2019-10-02T19:37:32Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

mw-ding · 2019-10-02T20:11:19Z

cc @fantapsody @spacedragon @zfy0701 to be aware of the config service change.

elasticmachine · 2019-10-02T21:20:37Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: ca405bc

mshustov

let's disable the app by default to keep the current behaviour and feel free to merge after

…46664) (#47253) * [Code] code backend NP migration * Move code plugin config to NP config service * Move code plugin logger to NP logger service * minor type error * remove joi config * addressing comments * fix unit tests * Migrate to xpack feature service for NP * minor comments * fix code mocha test scripts * fix type * fix i18nrc.json for code

mw-ding added the Team:Code label Sep 26, 2019

mw-ding requested a review from joshdover September 27, 2019 20:05

mw-ding requested a review from mshustov September 27, 2019 22:18

mw-ding added v7.5.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 27, 2019

mw-ding changed the title ~~[Code] code backend NP migration~~ [Code] migrate Code config and logging service to new platform Sep 27, 2019

mw-ding marked this pull request as ready for review September 27, 2019 23:12

mshustov reviewed Sep 30, 2019

View reviewed changes

mw-ding commented Sep 30, 2019

View reviewed changes

mw-ding commented Oct 1, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

mshustov reviewed Oct 2, 2019

View reviewed changes

mw-ding added 9 commits October 2, 2019 10:22

[Code] code backend NP migration

a5fdea7

Move code plugin config to NP config service

27050be

Move code plugin logger to NP logger service

e93897d

minor type error

18232b6

remove joi config

2d9d3c0

addressing comments

7857507

fix unit tests

fa61a71

Migrate to xpack feature service for NP

ee79589

minor comments

d91610c

fix code mocha test scripts

6713387

mw-ding changed the title ~~[Code] migrate Code config and logging service to new platform~~ [Code] migrate Code config/logging/feature service to new platform Oct 2, 2019

fix type

a2c8740

This comment has been minimized.

Sign in to view

fix i18nrc.json for code

ca405bc

mw-ding requested review from fantapsody, spacedragon and zfy0701 October 2, 2019 20:10

mw-ding requested a review from mshustov October 2, 2019 20:11

mshustov approved these changes Oct 3, 2019

View reviewed changes

mw-ding merged commit 8bdb20a into elastic:master Oct 3, 2019

mw-ding mentioned this pull request Oct 3, 2019

[7.x] [Code] migrate Code config/logging/feature service to new platform (#46664) #47253

Merged

mw-ding mentioned this pull request Oct 3, 2019

[Code] Embedded Code Snippet Component #47183

Merged

7 tasks

mw-ding added the Feature:New Platform label Oct 22, 2019

Conversation

mw-ding commented Sep 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Sep 26, 2019

Uh oh!

elasticmachine commented Sep 26, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 27, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 27, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 28, 2019

💔 Build Failed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mw-ding left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mw-ding commented Sep 30, 2019

Uh oh!

elasticmachine commented Sep 30, 2019

💔 Build Failed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Oct 1, 2019

💔 Build Failed

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mw-ding commented Sep 26, 2019 •

edited

Loading

mshustov left a comment •

edited

Loading