[grafana] Improve dashboards download script #3350
Open
+15
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xinity
requested review from
jkroepke,
maorfr,
torstenwalter,
Xtigyro and
zanhsieh
as code owners
Signed-off-by: Rachid Zarouali <[email protected]>
xinity
force-pushed
the
feature/improve-grafana-download-dashboards
branch
from
9cda7f8
to
b18764b
Compare
|
Please bump the chart version, thanks! |
xinity
changed the title
Signed-off-by: Rachid Zarouali <[email protected]>
|
@jkroepke oups, missed this, done :) |
jkroepke
reviewed
Oct 10, 2024
charts/grafana/templates/_config.tpl
Outdated
| @@ -81,7 +81,14 @@ download_dashboards.sh: | | |||
| {{- range $provider, $dashboards := .Values.dashboards }} | |||
| {{- range $key, $value := $dashboards }} | |||
| {{- if (or (hasKey $value "gnetId") (hasKey $value "url")) }} | |||
| curl -skf \ | |||
| {{- if $value.url }} | |||
There was a problem hiding this comment.
I guess, if someone changes the URL logic below, this may get forgotten. Not sure, if it make sense to move the Source URL logic to a dedicated template helper.
WDYT?
There was a problem hiding this comment.
@jkroepke i assume that if the logic is changed then lines 120 and below will also be changed right ? :
not sure for now it worth it to move it into a helper template even though it would better in the future :)
There was a problem hiding this comment.
Maybe @zanhsieh has an opinion here.
jkroepke
approved these changes
Oct 10, 2024
zanhsieh
reviewed
Oct 13, 2024
charts/grafana/templates/_config.tpl
Outdated
Comment on lines
85
to
89
| {{- if $value.url }} | ||
| echo "Source Url: {{ $value.url }}" | ||
| {{- else }} | ||
| echo "Source Url: https://grafana.com/api/dashboards/{{ $value.gnetId }}/revisions/{{- if $value.revision -}}{{ $value.revision }}{{- else -}}1{{- end -}}/download" | ||
| {{- end }} |
There was a problem hiding this comment.
Suggested change
| {{- if $value.url }} | |
| echo "Source Url: {{ $value.url }}" | |
| {{- else }} | |
| echo "Source Url: https://grafana.com/api/dashboards/{{ $value.gnetId }}/revisions/{{- if $value.revision -}}{{ $value.revision }}{{- else -}}1{{- end -}}/download" | |
| {{- end }} | |
| echo "Source Url: {{ include "grafana.url" . }} |
|
please fix CI failures and MR conflicts |
|
@jkroepke will squash to improve readability |
|
@xinity |
move the download message out of the if loop Signed-off-by: Rachid Zarouali <[email protected]>
Signed-off-by: Rachid Zarouali <[email protected]>
improvment thanks to community feedback, need some testing Signed-off-by: Rachid Zarouali <[email protected]>
Signed-off-by: Rachid Zarouali <[email protected]>
Signed-off-by: Tero Paloheimo <[email protected]>
Signed-off-by: Charles Korn <[email protected]>
Signed-off-by: Charles Korn <[email protected]>
Signed-off-by: Charles Korn <[email protected]>
Signed-off-by: Charles Korn <[email protected]>
Signed-off-by: Charles Korn <[email protected]>
Signed-off-by: Rachid Zarouali <[email protected]>
Signed-off-by: Rachid Zarouali <[email protected]>
xinity
force-pushed
the
feature/improve-grafana-download-dashboards
branch
from
6c00445
to
850d017
Compare
Signed-off-by: Rachid Zarouali <[email protected]>
|
@zanhsieh too many commits but signed and fix :) |
jkroepke
reviewed
Oct 15, 2024
add improvment suggested Co-authored-by: Jan-Otto Kröpke <[email protected]> Signed-off-by: Rachid Zarouali <[email protected]>
jkroepke
reviewed
Oct 16, 2024
| {{/* | ||
| Define the source URL | ||
| */}} | ||
| {{- define "grafana.url" -}} |
There was a problem hiding this comment.
Suggested change
| {{- define "grafana.url" -}} | |
| {{- define "grafana.dashboard-dowload-url" -}} |
| @@ -81,7 +81,9 @@ download_dashboards.sh: | | |||
| {{- range $provider, $dashboards := .Values.dashboards }} | |||
| {{- range $key, $value := $dashboards }} | |||
| {{- if (or (hasKey $value "gnetId") (hasKey $value "url")) }} | |||
| curl -skf \ | |||
| echo "downloading: {{ $key }} dashboard" | |||
| echo "Source Url: {{ include "grafana.url" . }} | |||
There was a problem hiding this comment.
Suggested change
| echo "Source Url: {{ include "grafana.url" . }} | |
| echo "Source URL: {{ include "grafana.dashboard-dowload-url" . }} |
| @@ -224,6 +224,16 @@ Formats imagePullSecrets. Input is (dict "root" . "imagePullSecrets" .{specific | |||
| {{- $secretFound}} | |||
| {{- end -}} | |||
|
|
|||
|
|
|||
| {{/* | |||
| Define the source URL | |||
There was a problem hiding this comment.
Please also add the template to the curl command!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims at improving the logging experience of the dowload dashboards script to :