Skip to content

TSVB display interval information when building - #32117

Merged
alexwizp merged 6 commits into
elastic:masterfrom
alexwizp:31590
Mar 7, 2019
Merged

TSVB display interval information when building#32117
alexwizp merged 6 commits into
elastic:masterfrom
alexwizp:31590

Conversation

@alexwizp

@alexwizp alexwizp commented Feb 27, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Fix: #31590

Added panel interval for the Metric, Top N, Gauge and Markdown tabs

New behavior:
31590_1

Checklist

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

For maintainers

@elastic elastic deleted a comment from elasticmachine Feb 28, 2019
@elastic elastic deleted a comment from elasticmachine Feb 28, 2019
@elastic elastic deleted a comment from elasticmachine Feb 28, 2019
@elastic elastic deleted a comment from elasticmachine Feb 28, 2019
@elasticmachine

elasticmachine commented Feb 28, 2019

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@alexwizp

alexwizp commented Feb 28, 2019

Copy link
Copy Markdown
Contributor Author

@AlonaNadler Could you please review?

markov00
markov00 requested changes Mar 1, 2019
View reviewed changes
Comment thread src/legacy/core_plugins/metrics/public/components/vis_editor_visualization.js Outdated
Comment thread src/legacy/core_plugins/metrics/public/components/vis_editor_visualization.js Outdated
Comment thread src/legacy/core_plugins/metrics/public/components/vis_editor_visualization.js Outdated
Comment thread src/legacy/core_plugins/metrics/public/components/vis_editor_visualization.js Outdated
Comment thread x-pack/plugins/translations/translations/zh-CN.json Outdated
flash1293
flash1293 requested changes Mar 1, 2019
View reviewed changes
Comment thread src/legacy/core_plugins/metrics/public/components/vis_editor_visualization.js Outdated
@alexwizp
alexwizp force-pushed the 31590 branch 2 times, most recently from 2ae33c0 to 071b0d6 Compare March 4, 2019 11:09
@elasticmachine

elasticmachine commented Mar 4, 2019

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@alexwizp
alexwizp requested review from flash1293 and markov00 March 4, 2019 12:05
@flash1293

flash1293 commented Mar 6, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

@alexwizp Interval is displayed everywhere correctly, so this part LGTM. However I'm not 100% sure if this is what we need here - an interval of >= 1d doesn't tell us much about the time span the metric is calculated for. Is there a way to "resolve" the >= 1d value to the actual interval used (as it is done for auto)? If there is no easy way to do so the current approach is fine.

@alexwizp

alexwizp commented Mar 6, 2019

Copy link
Copy Markdown
Contributor Author

@flash1293 Hm... looks like you are right! For >= we should show the actual interval. I'll update my PR

@elasticmachine

elasticmachine commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@flash1293

flash1293 commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

Tested and works fine for me - but as I'm looking at it, doesn't it always make sense to display the actual interval instead of interpreting the user setting? This should always be correct, no matter what, right?

@alexwizp

alexwizp commented Mar 6, 2019

Copy link
Copy Markdown
Contributor Author

@flash1293 we cannot. Please see the test case below:

image

@flash1293

flash1293 commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

@alexwizp Makes sense, good work 👍

@elasticmachine

elasticmachine commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

💔 Build Failed

@alexwizp

alexwizp commented Mar 6, 2019

Copy link
Copy Markdown
Contributor Author

retest

@elasticmachine

elasticmachine commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

💔 Build Failed

@alexwizp

alexwizp commented Mar 6, 2019

Copy link
Copy Markdown
Contributor Author

retest

@elasticmachine

elasticmachine commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@alexwizp
alexwizp merged commit 787bdb8 into elastic:master Mar 7, 2019
alexwizp added a commit to alexwizp/kibana that referenced this pull request Mar 7, 2019
@alexwizp
TSVB display interval information when building (elastic#32117)
f6e6453
@alexwizp alexwizp mentioned this pull request Mar 7, 2019
Merged
alexwizp added a commit to alexwizp/kibana that referenced this pull request Mar 7, 2019
@alexwizp
TSVB display interval information when building (elastic#32117)
68f132a
@alexwizp alexwizp mentioned this pull request Mar 7, 2019
Merged
alexwizp added a commit that referenced this pull request Mar 7, 2019
@alexwizp
TSVB display interval information when building (#32117) (#32624)
89c1c58
alexwizp added a commit that referenced this pull request Mar 7, 2019
@alexwizp
TSVB display interval information when building (#32117) (#32623)
793b8f6
PhilippBaranovskiy pushed a commit to PhilippBaranovskiy/kibana that referenced this pull request Mar 7, 2019
@alexwizp @PhilippBaranovskiy
TSVB display interval information when building (elastic#32117)
c93e170
@alexwizp
alexwizp deleted the 31590 branch January 4, 2020 08:13
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
@alexwizp
TSVB display interval information when building (elastic#32117)
f25cc1d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markov00 markov00 markov00 approved these changes
@flash1293 flash1293 flash1293 approved these changes
@timroes timroes Awaiting requested review from timroes
@sulemanof sulemanof Awaiting requested review from sulemanof

Assignees

No one assigned

Labels

Feature:TSVB TSVB (Time Series Visual Builder) release_note:enhancement v7.0.0 v7.2.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alexwizp @elasticmachine @flash1293 @markov00