feat: implement webhook_url_file for discord and msteams #3555
Conversation
implements prometheus#3482 Signed-off-by: Philipp Born <git@pborn.eu>
implements prometheus#3536 Signed-off-by: Philipp Born <git@pborn.eu>
|
We would love to have this feature in the next release, @simonpasquier / @gotjosh could you maybe take a look? |
| if n.conf.WebhookURL != nil { | ||
| url = n.conf.WebhookURL.String() | ||
| } else { | ||
| content, err := os.ReadFile(n.conf.WebhookURLFile) |
There was a problem hiding this comment.
A bit of a nit, but perhaps call this b instead of content. Two reasons:
- It's short.
- It's a somewhat idiomatic way of telling other maintainers that this variable is a byte slice instead of a string without having to check the docs for
os.ReadFile.
| content, err := os.ReadFile(n.conf.WebhookURLFile) | |
| b, err := os.ReadFile(n.conf.WebhookURLFile) |
|
|
||
| f, err := os.CreateTemp("", "webhook_url") | ||
| require.NoError(t, err, "creating temp file failed") | ||
| _, err = f.WriteString(u.String() + "\n") |
There was a problem hiding this comment.
Good test, to check for removing whitespace! 👍
| if n.conf.WebhookURL != nil { | ||
| url = n.conf.WebhookURL.String() | ||
| } else { | ||
| content, err := os.ReadFile(n.conf.WebhookURLFile) |
There was a problem hiding this comment.
| content, err := os.ReadFile(n.conf.WebhookURLFile) | |
| b, err := os.ReadFile(n.conf.WebhookURLFile) |
|
@tamcore Do you have time to implement the proposed changes? |
|
@dbluxo early next week maybe. If there'd be an official word about approval. But it should maybe also be considered, that the current code more or less lines up with the other receiver implementations. The proposed changes seem to make sense - but I feel like having it uniform might be preferable. |
|
@tamcore Thanks for your response, I think being consistent with the other receivers is a valid approach. |
There was a problem hiding this comment.
LGTM
Thank you so much for your contribution this looks great - @grobinson-grafana comments are very valid, but as you said this aligns with other notifiers so I'm good with this.
…3555) * feat: implement webhook_url_file for discord implements prometheus#3482 Signed-off-by: Philipp Born <git@pborn.eu> * feat: implement webhook_url_file for msteams implements prometheus#3536 Signed-off-by: Philipp Born <git@pborn.eu> --------- Signed-off-by: Philipp Born <git@pborn.eu> Signed-off-by: Gokhan Sari <gokhan@sari.me>
Closes #3536 and #3482.