Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TSVB display interval information when building #32117

Merged
merged 6 commits into from
Mar 7, 2019
Merged

TSVB display interval information when building #32117

merged 6 commits into from
Mar 7, 2019

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Feb 27, 2019 •
edited
Loading

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
Copy link
Contributor

💚 Build Succeeded

@alexwizp
Copy link
Contributor Author

@AlonaNadler Could you please review?

@alexwizp alexwizp force-pushed the 31590 branch 2 times, most recently from 2ae33c0 to 071b0d6 Compare March 4, 2019 11:09
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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

flash1293 commented Mar 6, 2019 •
edited
Loading

@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
Copy link
Contributor Author

alexwizp commented Mar 6, 2019

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

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@flash1293
Copy link
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
Copy link
Contributor Author

alexwizp commented Mar 6, 2019

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

image

@flash1293
Copy link
Contributor

@alexwizp Makes sense, good work 👍

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alexwizp
Copy link
Contributor Author

alexwizp commented Mar 6, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@alexwizp
Copy link
Contributor Author

alexwizp commented Mar 6, 2019

retest

@elasticmachine
Copy link
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flash1293 flash1293 flash1293 approved these changes

@markov00 markov00 markov00 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