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.

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

Merged
merged 21 commits into from
Feb 8, 2021
Merged

Conversation

ebadyano
Copy link
Contributor

This commit changes the chart generator output to ndjson and addresses validation errors for child "query" on kibana 7.x.

@ebadyano ebadyano added enhancement Improves the status quo :misc Changes that don't affect users directly: linter fixes, test improvements, etc. blocked This item cannot be finished because of a dependency breaking Non-backwards compatible change labels Oct 16, 2020
@ebadyano ebadyano added this to the 2.0.2 milestone Oct 16, 2020
@ebadyano
Copy link
Contributor Author

This should only be merged once we upgrade our metric store to. 7.x.

@ebadyano ebadyano self-assigned this Oct 16, 2020
@danielmitterdorfer danielmitterdorfer modified the milestones: 2.0.2, 2.0.3 Oct 26, 2020
@DJRickyB DJRickyB modified the milestones: 2.0.3, 2.x Jan 11, 2021
@ebadyano
Copy link
Contributor Author

ebadyano commented Feb 1, 2021

@danielmitterdorfer @dliappis could you please review? Thank you!

@dliappis
Copy link
Contributor

dliappis commented Feb 1, 2021

@danielmitterdorfer @dliappis could you please review? Thank you!

could you fix the failing PR checks first? Thanks!

@ebadyano
Copy link
Contributor Author

ebadyano commented Feb 1, 2021

Sorry, fixed the lint issues.

@ebadyano ebadyano removed blocked This item cannot be finished because of a dependency breaking Non-backwards compatible change labels Feb 1, 2021
Copy link
Member

@danielmitterdorfer danielmitterdorfer left a 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:
Copy link
Member

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.

@@ -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),
Copy link
Member

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?

@danielmitterdorfer danielmitterdorfer modified the milestones: 2.x, 2.1.0 Feb 3, 2021
@danielmitterdorfer
Copy link
Member

@elasticmachine test this please

Copy link
Member

@danielmitterdorfer danielmitterdorfer left a 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.

@@ -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}"
Copy link
Member

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.

@ebadyano
Copy link
Contributor Author

ebadyano commented Feb 8, 2021

@danielmitterdorfer thank you for the review!

@ebadyano ebadyano merged commit 23790e1 into master Feb 8, 2021
@ebadyano ebadyano deleted the kibana-chart branch March 31, 2021 16:21
@ebadyano ebadyano added :internal Changes for internal, undocumented features: e.g. experimental, release scripts and removed :misc Changes that don't affect users directly: linter fixes, test improvements, etc. labels Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves the status quo :internal Changes for internal, undocumented features: e.g. experimental, release scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants