Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing fields for Opsgenie integration #339

Merged
merged 1 commit into from
Oct 5, 2024

Conversation

SLASHLogin
Copy link
Contributor

Added missing fields to the Opsgenie integration, enabling the proper configuration of most fields. With this PR, issue #62 can be closed, and #329 will be superseded. If the changes from #329 are preferred, I can rebase or cherry-pick them.

Added fields:

  • actions: Custom actions that will be available for the alert.
  • tags: Tags of the alert.
  • visibleTo: Teams and users that the alert will become visible to without sending any notification.
  • details: Map of key-value pairs to use as custom properties of the alert.
  • entity: Entity field of the alert that is generally used to specify which domain the alert is related to.
  • user: Display name of the request owner.

@pasha-codefresh
Copy link
Member

@CodiumAI-Agent /review

@CodiumAI-Agent
Copy link

CodiumAI-Agent commented Oct 5, 2024

PR Reviewer Guide 🔍

(Review updated until commit f2ecd97)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling
Ensure that the error handling is consistent and comprehensive, particularly in the templating and parsing sections where multiple points of failure could occur.

Code Complexity
The templating logic for various fields like VisibleTo, Details, Actions, and Tags is quite complex and could benefit from refactoring to improve maintainability and readability.

Performance Concern
The implementation of templating for each field in a loop may lead to performance issues, especially with a large number of items in fields like Actions and VisibleTo.

@pasha-codefresh
Copy link
Member

Thank you! Amazing work

@pasha-codefresh pasha-codefresh merged commit 22ccfe0 into argoproj:master Oct 5, 2024
3 checks passed
@CodiumAI-Agent
Copy link

Persistent review updated to latest commit f2ecd97

@pasha-codefresh
Copy link
Member

Please open PR into ArgoCD with notification-engine dependency update. You can ping me in PR, i will help with review

@pasha-codefresh
Copy link
Member

@SLASHLogin

@SLASHLogin
Copy link
Contributor Author

Thanks a lot for the review!

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.

3 participants