Skip to content

opsgenie : trim space for api key file#4195

Merged
gotjosh merged 3 commits intoprometheus:mainfrom
zoov-xavier:opsgenie_trim_api_key_file
Mar 6, 2025
Merged

opsgenie : trim space for api key file#4195
gotjosh merged 3 commits intoprometheus:mainfrom
zoov-xavier:opsgenie_trim_api_key_file

Conversation

@zoov-xavier
Copy link
Contributor

The goal is to trim the spaces on the API key file for Opsgenie as it is already done for most other integrations (Slack, Discord, PagerDuty, etc.)

@grobinson-grafana
Copy link
Collaborator

LGTM. Could you please write a unit test for this too?

@zoov-xavier zoov-xavier force-pushed the opsgenie_trim_api_key_file branch 4 times, most recently from 143e02f to cd1de9c Compare January 7, 2025 13:27
@zoov-xavier
Copy link
Contributor Author

Hello, @grobinson-grafana thanks for the review. I added some tests. Let me know if any updates are needed.

@zoov-xavier zoov-xavier force-pushed the opsgenie_trim_api_key_file branch from cd1de9c to 59b0495 Compare February 13, 2025 13:46
Signed-off-by: zoov-xavier <xavier.jeanneney@fifteen.eu>
@zoov-xavier zoov-xavier force-pushed the opsgenie_trim_api_key_file branch from 59b0495 to f621e92 Compare February 26, 2025 07:47
@zoov-xavier
Copy link
Contributor Author

Hello, @grobinson-grafana
I rebased the PR maybe you could merge it ?
Thanks !

notifierWithUpdate, err := New(&opsGenieConfigWithUpdate, tmpl, promslog.NewNopLogger())

require.NoError(t, err)
requests, _, err := notifierWithUpdate.createRequests(ctx)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a lint error here, as err is not checked:

ineffectual assignment to err (ineffassign)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have addressed this in c1379bd

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have addressed this in c1379bd

Copy link
Member

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gotjosh gotjosh merged commit 5fecfde into prometheus:main Mar 6, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants