[Timeseries] remove unused configuration properties #62543

alexwizp · 2020-04-04T08:29:44Z

Dev-Docs

Mark chartResolution and minimumBucketSize as deprecated configuration properties for TSVB 👋

In #9725 TSVB visualization was added to Kibana. Also in this PR we can find that 2 configuration properties were declared: chartResolution and minimumBucketSize. The problem is that until today no one has started using these properties, an implementation has not been added

elasticmachine · 2020-04-04T08:42:22Z

Pinging @elastic/kibana-app (Team:KibanaApp)

flash1293 · 2020-04-06T08:40:39Z

I don't feel comfortable removing these in a minor version - if an installation still has these settings set, Kibana won't start for them. My suggestion:

Move them to the NP timeseries plugin: https://github.com/elastic/kibana/blob/master/src/plugins/vis_type_timeseries/server/index.ts#L25
use the depreactions helper to first rename them from metrics.* to vis_type_timeseries.* using the "silent" option and then mark them them as deprecated using the unused helper (example for silent renaming: https://github.com/elastic/kibana/blob/master/src/plugins/kibana_legacy/server/index.ts#L31 example for unused marking:

kibana/src/plugins/console/server/index.ts

Line 29 in 304b322

deprecations: ({ unused }) => [unused('ssl')],

)
Leave a TODO to remove with 8.0

This will have the following effect when a user still has these settings: Kibana will still start but log a warning that the setting is unused. With 8.0 we can remove the setting completely and add it to the breaking changes list.

alexwizp · 2020-04-06T17:23:03Z

@elasticmachine merge upstream

cjcenizal

Tested the changes to the Rollup plugin locally, code LGTM.

flash1293 · 2020-04-06T21:18:13Z

This test failure is probably legit, the infra app has a dependency on the metrics/vis_type_timeseries plugin. The reference has to be renamed.

alexwizp · 2020-04-07T15:33:00Z

ping @elastic/logs-metrics-ui

alexwizp · 2020-04-08T18:46:06Z

@elasticmachine merge upstream

# Conflicts: # x-pack/legacy/plugins/rollup/kibana.json

flash1293 · 2020-04-09T12:25:52Z

I tested this PR and it looks and works fine - one thing I noticed though is that if you just use visTypeTimeseries.enabled: false, then the server will be gone but the visualization will still show up because it's still in the legacy plugin (you would have to use it along with metrics.enabled: false to disable everything).

@alexwizp what do you think about parking this PR till we moved the rest of timeseries over? This will very likely happen next week as there are no more blockers.

alexwizp · 2020-04-09T12:56:40Z

@flash1293 np, let's parking it =)

# Conflicts: # src/legacy/core_plugins/vis_type_timeseries/index.ts # x-pack/legacy/plugins/rollup/server/plugin.ts

kibanamachine · 2020-04-15T18:07:23Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 90c6b05

History

💛 Build #39808 was flaky 0561f2a
💚 Build #39705 succeeded 586c347
💚 Build #39242 succeeded d576423
💔 Build #39201 failed f1e04d4
💔 Build #39161 failed e666e93

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

flash1293

LGTM, bye bye metrics plugin

Zacqary

Looks good from the infra side

* [Timeseries] remove unused configuration properties * Fix PR comments * update id of vis_type_timeseries plugin * metrics -> vis_type_timeseries * fix wrong plugin id * update requiredPliugins for infra/kibana.json * change id * update plugin id in infra folder Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* [Timeseries] remove unused configuration properties * Fix PR comments * update id of vis_type_timeseries plugin * metrics -> vis_type_timeseries * fix wrong plugin id * update requiredPliugins for infra/kibana.json * change id * update plugin id in infra folder Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

[Timeseries] remove unused configuration properties

ebd3be8

alexwizp requested review from VladLasitsa and flash1293 April 4, 2020 08:41

alexwizp self-assigned this Apr 4, 2020

alexwizp added Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels Apr 4, 2020

alexwizp added v7.8.0 v8.0.0 release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. labels Apr 4, 2020

alexwizp added 2 commits April 6, 2020 13:14

Fix PR comments

78f1719

Merge remote-tracking branch 'upstream/master' into config_tsvb

842990d

alexwizp force-pushed the config_tsvb branch from f79bcec to 842990d Compare April 6, 2020 10:15

alexwizp marked this pull request as ready for review April 6, 2020 12:22

alexwizp added 2 commits April 6, 2020 16:51

update id of vis_type_timeseries plugin

67d4b3c

metrics -> vis_type_timeseries

5c3fb68

alexwizp requested a review from a team as a code owner April 6, 2020 15:43

Merge branch 'master' into config_tsvb

6d36335

cjcenizal self-requested a review April 6, 2020 17:47

cjcenizal approved these changes Apr 6, 2020

View reviewed changes

alexwizp added 2 commits April 6, 2020 22:25

fix wrong plugin id

b74e4e3

Merge remote-tracking branch 'upstream/master' into config_tsvb

cd6482d

update requiredPliugins for infra/kibana.json

e666e93

alexwizp requested a review from a team as a code owner April 7, 2020 07:03

alexwizp added 3 commits April 7, 2020 13:26

change id

f1e04d4

Merge remote-tracking branch 'upstream/master' into config_tsvb

ba22d70

update plugin id in infra folder

d576423

alexwizp mentioned this pull request Apr 7, 2020

Index pattern management plugin - src/legacy/core_plugins/management => new platform plugin #62594

Merged

1 task

timroes mentioned this pull request Apr 8, 2020

Fail startup with unknown optionalPlugins dependency in dev mode #62937

Open

elasticmachine and others added 2 commits April 8, 2020 14:46

Merge branch 'master' into config_tsvb

586c347

Merge remote-tracking branch 'upstream/master' into config_tsvb

0561f2a

# Conflicts: # x-pack/legacy/plugins/rollup/kibana.json

Merge remote-tracking branch 'upstream/master' into config_tsvb

90c6b05

# Conflicts: # src/legacy/core_plugins/vis_type_timeseries/index.ts # x-pack/legacy/plugins/rollup/server/plugin.ts

alexwizp requested a review from a team April 15, 2020 16:26

flash1293 approved these changes Apr 16, 2020

View reviewed changes

Zacqary approved these changes Apr 16, 2020

View reviewed changes

alexwizp merged commit 871f720 into elastic:master Apr 16, 2020

alexwizp mentioned this pull request Apr 16, 2020

[7.x] [Timeseries] remove unused configuration properties (#62543) #63728

Merged

[Timeseries] remove unused configuration properties #62543

[Timeseries] remove unused configuration properties #62543

Uh oh!

Conversation

alexwizp commented Apr 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dev-Docs

Uh oh!

elasticmachine commented Apr 4, 2020

Uh oh!

flash1293 commented Apr 6, 2020

Uh oh!

alexwizp commented Apr 6, 2020

Uh oh!

cjcenizal left a comment

Choose a reason for hiding this comment

Uh oh!

flash1293 commented Apr 6, 2020

Uh oh!

alexwizp commented Apr 7, 2020

Uh oh!

alexwizp commented Apr 8, 2020

Uh oh!

flash1293 commented Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexwizp commented Apr 9, 2020

Uh oh!

kibanamachine commented Apr 15, 2020

💚 Build Succeeded

History

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

Zacqary left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

alexwizp commented Apr 4, 2020 •

edited

Loading

flash1293 commented Apr 9, 2020 •

edited

Loading