-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
feat: Add Zabbix credential only node #11489
Conversation
Hey @adina-hub, Thanks for the PR, We have created "GHC-371" as the internal reference to get this reviewed. One of us will be in touch if there are any changes needed, in most cases this is normally within a couple of weeks but it depends on the current workload of the team. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some notes, As the ZabbixAuthApi
is using a deprecated method we should probably skip it and only move forward with the API token optoin.
@adina-hub the credential test still returns a success if the toke is incorrect or missing. Everything else looks good though, I have been testing against a self hosted Zabbix instance and Zabbix Cloud. |
@Joffcom Last changes are pushed, credentials were tested again, now it works. |
@adina-hub that is perfect thanks 🙏🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good thanks.
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
Got released with |
Summary
Related Linear tickets, Github issues, and Community forum posts
Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)