[vega] Handle removal of deprecated date histogram interval #109090

alexwizp · 2021-08-18T12:32:47Z

Summary

This PR is created as a sample solution for #106352. I suggest, implicitly for the user, to change the interval field to the value corresponding to the current interval. Other visualizations work the same way.

Plus, this solution is backward compatible with older vega specs. We shouldn't think about the complex solutions described in #106352 (comment)

What options are supported:

{ ... , fixed_interval: string }
{..., calendar_interval: string }
{..., interval: string}
{..., interval: {%autointerval%: true }}
{..., interval: {%autointerval%: number }}

The last three are automatically converted to fixed_interval / calendar_interval

Fixes: elastic#106352

alexwizp · 2021-08-19T09:32:34Z

@elasticmachine merge upstream

stratoula · 2021-08-20T11:44:06Z

@alexwizp there are some cases that we are not bwc, for example, if I add as interval "month" or "day".

elasticmachine · 2021-08-26T09:41:04Z

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

alexwizp · 2021-08-27T14:48:09Z

@elasticmachine merge upstream

alexwizp · 2021-08-30T08:22:00Z

@elasticmachine merge upstream

ppisljar

app services changes LGTM

src/plugins/vis_type_vega/public/components/vega_info_message.tsx

VladLasitsa

LGTM!

stratoula · 2021-08-31T13:34:02Z

cc @ghudgins this is not so urgent as it will be merged for 8.0 but maybe you want to play with it

alexwizp · 2021-09-20T07:58:19Z

@elasticmachine merge upstream

stratoula

It works fine and we keep this interval and auto_interval logic. By keeping this, it means that we support something that is not supported anymore by ES (the interval param). We have discussed it with the team and we prefer this solution rather than not being bwc. (maybe it makes sense a brief mention in our docs)

My only concern is the usage of auto_date_histogram in the default spec. As soon as we are keeping the interval setting, I think the default spec should stay as it is and provide an example of the auto_interval usage (explicit vega variable). Another reason to not use the auto_date_histogramin our default spec is that is doesn't support yet extended_bounds.

From the other hand, If we don't want to "advertise" the interval setting then the default spec should use the auto_date_histogram.

I don't have a strong opinion tbh. I would like your feedback on this one @flash1293 @ghudgins.

stratoula · 2021-09-21T11:02:40Z

Synced offline with the team, we will proceed with the existing default spec that uses the interval and extended_bounds properties.

stratoula · 2021-09-21T12:49:57Z

@alexwizp can we also unskip the vega functional tests #106657?

stratoula

Code LGTM, tested it locally in Chrome. I think we have done the best we could here.
The only thing remaining is to mention this in vega docs but I can do it on a separate PR

alexwizp · 2021-09-22T12:01:34Z

@elasticmachine merge upstream

kibanamachine · 2021-09-22T14:13:21Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`visTypeVega`	240	242	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`visTypeVega`	1.9MB	1.9MB	+111.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`data`	492.3KB	492.5KB	+186.0B
`visTypeVega`	35.8KB	37.0KB	+1.2KB
total			+1.3KB

History

💚 Build #154893 succeeded adfd563
💔 Build #154597 failed 3e70181
💚 Build #154166 succeeded 561e27a
💚 Build #152237 succeeded 1752bb2
💔 Build #152190 failed afdcf77

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

cc @alexwizp

stratoula · 2021-09-22T14:29:54Z

@alexwizp I removed the v7.16 label, this is only for 8 :D

…109090) * [vega] Handle removal of deprecated date histogram interval Fixes: elastic#106352 * fix CI * add deprecation_interval_info * add test * Update vega_info_message.tsx * fix types * Update es_query_parser.ts * apply comments * fix error Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Uladzislau Lasitsa <vlad.lasitsa@gmail.com>

* [vega] Handle removal of deprecated date histogram interval (#109090) * [vega] Handle removal of deprecated date histogram interval Fixes: #106352 * fix CI * add deprecation_interval_info * add test * Update vega_info_message.tsx * fix types * Update es_query_parser.ts * apply comments * fix error Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Uladzislau Lasitsa <vlad.lasitsa@gmail.com> * Updates the VEGA docs for v8.0 (#112781) * Update VEGA docs for v8.0 * Update docs/user/dashboard/vega-reference.asciidoc Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co> Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> * [Vega] Replacing the 'interval' property should only happen for the date_histogram aggregation (#115001) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Uladzislau Lasitsa <vlad.lasitsa@gmail.com> Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co> Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co>

[vega] Handle removal of deprecated date histogram interval

27bd92e

Fixes: elastic#106352

Merge branch 'master' into 106352

fc71b48

fix CI

9187d9d

stratoula mentioned this pull request Aug 20, 2021

[vega] Handle removal of deprecated date histogram interval #106352

Closed

alexwizp added 3 commits August 25, 2021 13:04

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

f5675b5

add deprecation_interval_info

5bfad47

add test

4a64eb1

alexwizp marked this pull request as ready for review August 26, 2021 09:39

alexwizp requested a review from a team August 26, 2021 09:39

alexwizp requested a review from a team as a code owner August 26, 2021 09:39

alexwizp self-assigned this Aug 26, 2021

alexwizp added Feature:Vega Vega visualizations release_note:skip Skip the PR/issue when compiling release notes v7.16.0 v8.0.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Aug 26, 2021

Merge branch 'master' into 106352

015d885

alexwizp requested a review from VladLasitsa August 30, 2021 08:21

Merge branch 'master' into 106352

a0af3af

ppisljar approved these changes Aug 30, 2021

View reviewed changes

VladLasitsa suggested changes Aug 30, 2021

View reviewed changes

src/plugins/vis_type_vega/public/components/vega_info_message.tsx Outdated Show resolved Hide resolved

Update vega_info_message.tsx

e427692

VladLasitsa approved these changes Aug 30, 2021

View reviewed changes

Merge branch 'master' into 106352

1752bb2

Merge branch 'master' into 106352

561e27a

stratoula reviewed Sep 20, 2021

View reviewed changes

apply comments

3e70181

stratoula approved these changes Sep 21, 2021

View reviewed changes

stratoula mentioned this pull request Sep 22, 2021

Updates the VEGA docs for v8.0 #112781

Merged

fix error

adfd563

Merge branch 'master' into 106352

231b3bf

stratoula removed the v7.16.0 label Sep 22, 2021

stratoula added backport:skip This commit does not require backporting release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Sep 22, 2021

alexwizp merged commit 61410a3 into elastic:master Sep 22, 2021

rashmivkulkarni mentioned this pull request Sep 22, 2021

Unskip tests on 7.15 branch #110310

Closed

38 tasks

alexwizp mentioned this pull request Oct 14, 2021

[Vega] Replacing the 'interval' property should only happen for the date_histogram aggregation #115001

Merged

ThomThomson mentioned this pull request Nov 8, 2021

[Dashboard] [Functional Tests] Unskip Filtering Tests #117944

Merged

9 tasks

lukeelmers mentioned this pull request Jan 14, 2022

[7.17] Vega functional tests fail when run against ES 8.0 #123081

Closed

alexwizp mentioned this pull request Jan 17, 2022

[7.17] Vega functional tests fail when run against ES 8.0 #123134

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vega] Handle removal of deprecated date histogram interval #109090

[vega] Handle removal of deprecated date histogram interval #109090

alexwizp commented Aug 18, 2021 •

edited

Loading

alexwizp commented Aug 19, 2021

stratoula commented Aug 20, 2021

elasticmachine commented Aug 26, 2021

alexwizp commented Aug 27, 2021

alexwizp commented Aug 30, 2021

ppisljar left a comment

VladLasitsa left a comment

stratoula commented Aug 31, 2021

alexwizp commented Sep 20, 2021

stratoula left a comment •

edited

Loading

stratoula commented Sep 21, 2021

stratoula commented Sep 21, 2021

stratoula left a comment

alexwizp commented Sep 22, 2021

kibanamachine commented Sep 22, 2021

stratoula commented Sep 22, 2021

[vega] Handle removal of deprecated date histogram interval #109090

[vega] Handle removal of deprecated date histogram interval #109090

Conversation

alexwizp commented Aug 18, 2021 • edited Loading

Summary

alexwizp commented Aug 19, 2021

stratoula commented Aug 20, 2021

elasticmachine commented Aug 26, 2021

alexwizp commented Aug 27, 2021

alexwizp commented Aug 30, 2021

ppisljar left a comment

Choose a reason for hiding this comment

VladLasitsa left a comment

Choose a reason for hiding this comment

stratoula commented Aug 31, 2021

alexwizp commented Sep 20, 2021

stratoula left a comment • edited Loading

Choose a reason for hiding this comment

stratoula commented Sep 21, 2021

stratoula commented Sep 21, 2021

stratoula left a comment

Choose a reason for hiding this comment

alexwizp commented Sep 22, 2021

kibanamachine commented Sep 22, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

stratoula commented Sep 22, 2021

alexwizp commented Aug 18, 2021 •

edited

Loading

stratoula left a comment •

edited

Loading