Skip to content
This repository has been archived by the owner on Apr 26, 2021. It is now read-only.

remove Result from ruler grafana alert model #30

Closed

Conversation

domasx2
Copy link
Collaborator

@domasx2 domasx2 commented Mar 17, 2021

Heya,

GrafanaAlert model for ruler responses included some odd Result property that I think is not needed? It comes from https://github.com/grafana/grafana/blob/master/pkg/services/ngalert/models/models.go#L108. Updated to copy the model w/o the Result thingy

@domasx2 domasx2 requested review from papagian and owen-d March 17, 2021 07:32
@papagian
Copy link
Collaborator

The existing API used to include the created/updated alert definition to the response and that was the purpose of this property. This model will be renamed anyway.

@domasx2
Copy link
Collaborator Author

domasx2 commented Mar 17, 2021

The problem is that it shows up in grafana_alert model in GET return payload on the spec:
image

Do you mean it will be fixed in another way?

@papagian
Copy link
Collaborator

Do you mean it will be fixed in another way?

Yes, because we don't need this for the unified API but if you want merge it and I will open another PR for that (I will change it in the grafana repository and I will import it from there).

@domasx2
Copy link
Collaborator Author

domasx2 commented Mar 17, 2021

If it's gonna be fixed eventually, I'm good :) Will just close this then. Thanks!

@domasx2 domasx2 closed this Mar 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants