-
Notifications
You must be signed in to change notification settings - Fork 23.8k
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
New zabbix_service module #58926
New zabbix_service module #58926
Conversation
* new module zabbix_service * fix type * fix githubid
* new zabbix service module
The test
The test
|
1a55460
to
09d7bb0
Compare
@Akint @D3DeFi @K-dot @RedWhiteMiko @abulimov @akomic @cove @dj-wasabi @eikef @harrisongu @logan2211 @rubentsirunyan @sookido As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
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.
Tested against Zbx 3.0, 4.0 and 4.2. Works good. Only minor adjustements required.
Btw, do you plan to extend this module to support Service time and service dependencies as well?
- Algorithm used to calculate the sla | ||
required: false | ||
type: str | ||
choices: ["no", "one_child", "all_childs"] |
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.
Sorry for being neat-picky, but plural of child is children. Can you change this please? :)
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.
Also it is ideal not to refer to zabbix API documentation very much in module docs, it would be nice if you can add some notes about these choices (["no", "one_child", "all_childs"]
) in algorithm
description.
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.
Thank you for your review, I have applied your recommendations :)
It is a good idea, but I won't have any time to implement this in the next future. |
NP, maybe some day you, me or someone else will find a way to contribute this as a feature :) |
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.
Good work. Tested this
-label needs_triage
shipit
Co-Authored-By: sky-joker <megane@kurobuti.com>
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.
shipit
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.
Looked through the code.
shipit
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.
Same as #58051, please reimplement the state=dump into a dedicated info module
fae750c
to
7d299f7
Compare
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.
LGTM
@resmo is this good to go now? |
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.
LGTM
As a feature request, I would have returned the json of the api response: e.g. the json of the zabbix service created, or the json of the zabbix server deleted. But merge this and work towards.
SUMMARY
New module to manage Zabbix services.
ISSUE TYPE
COMPONENT NAME
zabbix_service
ADDITIONAL INFORMATION
Tested on Zabbix 4.0.