feat: allowing import dashboards from URL #43
Conversation
|
@jjaferson i think you need to rebase against the master branch to resolve the conflict |
|
@jjaferson please also update the dashboards documentation |
Dashboard will only be resolved from the url when changes are made to the url or json field in CR
keeping the json as fallback in case the resolving of the url fails
| valid, err := r.isJsonValid(d) | ||
| json := r.loadDashboardFromURL(d) | ||
| if json != "" { | ||
| d.Spec.Json = json |
There was a problem hiding this comment.
Wouldn't this still be overwriting the json field?
There was a problem hiding this comment.
Yes, but only if the downloaded json is valid. The only other way I can see of not overwriting the json is changing the function UpdateDashboard in the kubeHelper.go
There was a problem hiding this comment.
That's ok, maybe we can pass in the json instead of the whole dashboard to this function.
| @@ -209,6 +219,43 @@ func (r *ReconcileGrafanaDashboard) importDashboard(d *i8ly.GrafanaDashboard) (r | |||
| return reconcile.Result{}, err | |||
| } | |||
|
|
|||
| func (r *ReconcileGrafanaDashboard) loadDashboardFromURL(d *i8ly.GrafanaDashboard) string { | |||
There was a problem hiding this comment.
Would it make sense to return (string, error) here?
There was a problem hiding this comment.
I had the function returning errors at first but If an error occur while downloading a dashboard it won't stop dashboards from being imported as it will use the json field in the CR as a fallback, that is why I decided to just log the errors instead of return them.
I am not sure whether it's the best approach, let me know your thoughts on that.
There was a problem hiding this comment.
I'd rather return an error from this function and add an error check (and log) where it's called.
… into dashboard-from-url
Importing dashboards from a URL (in addition to json inside the CR)
https://issues.jboss.org/browse/INTLY-2983
Verification Steps