-
Notifications
You must be signed in to change notification settings - Fork 314
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.
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
Modify chart generator to support Kibana 7.x dashboards #1093
Conversation
This should only be merged once we upgrade our metric store to. 7.x. |
@danielmitterdorfer @dliappis could you please review? Thank you! |
could you fix the failing PR checks first? Thanks! |
Sorry, fixed the lint issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. I left a few comments / suggestions.
@@ -1587,7 +1587,7 @@ def bulk_tasks(self): | |||
task_names = [] | |||
for task in self.track.find_challenge_or_default(self.challenge).schedule: | |||
for sub_task in task: | |||
if sub_task.operation.type == track.OperationType.Bulk.to_hyphenated_string(): | |||
if track.OperationType.Bulk.to_hyphenated_string() in sub_task.operation.type: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this warrants at least a comment and maybe even an info message to the terminal on lenient matches.
esrally/chart_generator.py
Outdated
@@ -1033,7 +1033,7 @@ def segment_memory(title, environment, race_config): | |||
"seperate_axis": 1, | |||
"split_mode": "filters", | |||
"stacked": "none", | |||
"filter": "environment:{} AND track:{}".format(environment, race_config.track), | |||
"filter": "environment:{} AND track:\"{}\"".format(environment, race_config.track), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should change the .format()
invocations with f-strings in all places?
@elasticmachine test this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for iterating. I found one spot where we could simplify formatting a little but but apart from that LGTM. No need for another review round after addressing this.
esrally/chart_generator.py
Outdated
@@ -1484,7 +1471,7 @@ def generate_dashboard(chart_type, environment, track, charts, flavor=None): | |||
"y": (row * height), | |||
"w": chart_width, | |||
"h": height, | |||
"i": "{}".format(panelIndex) | |||
"i": f"{panelIndex}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A str(panelIndex)
should be sufficient here.
@danielmitterdorfer thank you for the review! |
This commit changes the chart generator output to ndjson and addresses validation errors for child "query" on kibana 7.x.