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

zabbix_*: zabbix-api dep clarification + removed unnecessary ZabbixAPIExtends class #53334

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
3 participants
@D3DeFi
Copy link
Contributor

D3DeFi commented Mar 5, 2019

SUMMARY

Unified zabbix_* modules under single zabbix-api dependency and removed thus unnecessary extend of ZabbixAPI class.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

zabbix_group
zabbix_group_facts
zabbix_host
zabbix_host_facts
zabbix_hostmacro
zabbix_maintenance
zabbix_map
zabbix_screen

ADDITIONAL INFORMATION

Zabbix modules are using ZabbixAPI class from zabbix-api python module by extending it and adding support for missing API calls. This was probably first introduced upon zabbix_host module creation sometimes around ~2014:

class ZabbixAPIExtends(ZabbixAPI):
    hostinterface = None

    def __init__(self, server, timeout, user, passwd, validate_certs, **kwargs):
        ZabbixAPI.__init__(self, server, timeout=timeout, user=user, passwd=passwd, validate_certs=validate_certs)
        self.hostinterface = ZabbixAPISubClass(self, dict({"prefix": "hostinterface"}, **kwargs))

By that time, zabbix-api did support only a few explicit API calls, but situation had changed at the end of 2015 where explicit API calls were replaced by universal call method in this commit.

This PR builds on this knowledge by first unifying all zabbix modules under specific python module dependency zabbix-api >= 0.5.3 and then removing unnecessary extending via ZabbixAPIExtends class.

Tested against all currently supported Zabbix server releases (3.0, 3.4, 4.0).

@D3DeFi

This comment has been minimized.

Copy link
Contributor Author

D3DeFi commented Mar 5, 2019

not sure if I should add doc fragment for this change as well.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 5, 2019

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

lib/ansible/modules/monitoring/zabbix/zabbix_template.py:329:15: undefined-variable Undefined variable 'ZabbixAPIException'
lib/ansible/modules/monitoring/zabbix/zabbix_template.py:437:15: undefined-variable Undefined variable 'ZabbixAPIException'
lib/ansible/modules/monitoring/zabbix/zabbix_template.py:496:11: undefined-variable Undefined variable 'ZabbixAPIException'

The test ansible-test sanity --test pep8 [explain] failed with 5 errors:

lib/ansible/modules/monitoring/zabbix/zabbix_group_facts.py:110:32: E127 continuation line over-indented for visual indent
lib/ansible/modules/monitoring/zabbix/zabbix_host.py:686:32: E127 continuation line over-indented for visual indent
lib/ansible/modules/monitoring/zabbix/zabbix_host_facts.py:171:32: E127 continuation line over-indented for visual indent
lib/ansible/modules/monitoring/zabbix/zabbix_hostmacro.py:190:32: E127 continuation line over-indented for visual indent
lib/ansible/modules/monitoring/zabbix/zabbix_screen.py:327:32: E127 continuation line over-indented for visual indent

click here for bot help

@ansibot

This comment has been minimized.

@D3DeFi D3DeFi force-pushed the D3DeFi:zabbix-modules-refactor branch from 61ada04 to fcebfc2 Mar 5, 2019

@D3DeFi

This comment has been minimized.

Copy link
Contributor Author

D3DeFi commented Mar 5, 2019

ready_for_review

@ansibot ansibot added core_review and removed needs_revision labels Mar 5, 2019

@bcoca

This comment has been minimized.

Copy link
Member

bcoca commented Mar 5, 2019

bot_status

@bcoca bcoca removed the needs_triage label Mar 5, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 5, 2019

Components

lib/ansible/modules/monitoring/zabbix/zabbix_group.py
support: community
maintainers: D3DeFi cove eikef harrisongu

lib/ansible/modules/monitoring/zabbix/zabbix_group_facts.py
support: core
maintainers: D3DeFi RedWhiteMiko eikef

lib/ansible/modules/monitoring/zabbix/zabbix_host.py
support: community
maintainers: D3DeFi cove dj-wasabi eikef harrisongu

lib/ansible/modules/monitoring/zabbix/zabbix_host_facts.py
support: core
maintainers: D3DeFi RedWhiteMiko eikef

lib/ansible/modules/monitoring/zabbix/zabbix_hostmacro.py
support: community
maintainers: D3DeFi cove eikef

lib/ansible/modules/monitoring/zabbix/zabbix_maintenance.py
support: community
maintainers: D3DeFi abulimov eikef

lib/ansible/modules/monitoring/zabbix/zabbix_map.py
support: core
maintainers: Akint D3DeFi eikef

lib/ansible/modules/monitoring/zabbix/zabbix_screen.py
support: community
maintainers: D3DeFi cove eikef harrisongu

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 1
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainers or core team members): D3DeFi
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.