Skip to content

Fixes for Zabbix module issues and compatibility fixes #62012

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

Merged

Conversation

t0fik
Copy link
Contributor

@t0fik t0fik commented May 1, 2022

ISSUES FIXED: #60795

What does this PR do?

  • Fixed errors on calling zabbix_user.admin_password_present state, due to changed error message in Zabbix 6.0
  • Fixed zabbix.host_update not mapping group ids list to list of dicts in format [{"groupid": groupid}, ...] ([BUG] zabbix_host.present groupid is missing #60795)
  • Fixed zabbix.user_update not mapping usergroup id list to list of dicts in format [{"usrgrpid": usrgrpid}, ...]
  • Added support for changed user object in Zabbix 5.4+
  • Added compatibility with Zabbix 4.0+ for zabbix.user_getmedia method
  • Added support for setting medias in zabbix.user_update for Zabbix 3.4+
  • Changed zabbix.user_get to return full user info with groups and medias
  • Changed zabbix.user_addmedia to return error with comment for Zabbix 4.0+ due to user.addmedia method removal
  • Changed zabbix.user_deletemedia to return error with comment for Zabbix 4.0+ due to user.deletemedia method removal
  • Added missing CLI Example sections

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@t0fik t0fik requested a review from a team as a code owner May 1, 2022 17:30
@t0fik t0fik requested review from garethgreenaway and removed request for a team May 1, 2022 17:30
@t0fik t0fik changed the title Zabbix 6.0 changed login error message Fixes for Zabbix module issues and compability fixes May 3, 2022
@t0fik t0fik force-pushed the fix/zabbix_user_admin_password branch from 238fa3e to aa6e9bc Compare May 3, 2022 15:09
@t0fik t0fik marked this pull request as draft May 5, 2022 06:44
@t0fik t0fik force-pushed the fix/zabbix_user_admin_password branch from 982d8da to e88651d Compare May 5, 2022 09:14
@t0fik t0fik changed the title Fixes for Zabbix module issues and compability fixes Fixes for Zabbix module issues and compatibility fixes May 6, 2022
@t0fik t0fik marked this pull request as ready for review May 6, 2022 08:36
@garethgreenaway
Copy link
Contributor

@t0fik Thanks! This looks fantastic! We have a number of open issues related to the Zabbix modules would you mind linking those issues to this PR so they'll get closed once this PR is merged? One additional ask, would you mind converting the tests over to PyTest? We can offer assistance if needed. Thanks again!

@garethgreenaway garethgreenaway added the Phosphorus v3005.0 Release code name and version label May 12, 2022
@garethgreenaway garethgreenaway added this to the Phosphorus v3005.0 milestone May 12, 2022
@t0fik
Copy link
Contributor Author

t0fik commented May 13, 2022

@garethgreenaway I've found only one issue, which is fixed by this PR.
I can take care about rest of them but I did not want to push to much code.
I did not review all functions in the module so not all bugs in zabbix module might be fixed, also refactorisation of the module is needed. I wanted to do that after merging this PR.
I've coverted tests today.

@github-actions
Copy link

Hi! I'm your friendly PR bot!

You might be wondering what I'm doing commenting here on your PR.

Yes, as a matter of fact, I am...

I'm just here to help us improve the documentation. I can't respond to
questions or anything, but what I can do, I do well!

Okay... so what do you do?

I detect modules that are missing docstrings or "CLI Example" on existing docstrings!
When I was created we had a lot of these. The documentation for these
modules need some love and attention to make Salt better for our users.

So what does that have to do with my PR?

I noticed that in this PR there are some files changed that have some of these
issues. So I'm leaving this comment to let you know your options.

Okay, what are they?

Well, my favorite, is that since you were making changes here I'm hoping that
you would be the most familiar with this module and be able to add some other
examples or fix any of the reported issues.

If I can, then what?

Well, you can either add them to this PR or add them to another PR. Either way is fine!

Well... what if I can't, or don't want to?

That's also fine! We appreciate all contributions to the Salt Project. If you
can't add those other examples, either because you're too busy, or unfamiliar,
or you just aren't interested, we still appreciate the contributions that
you've made already.

Whatever approach you decide to take, just drop a comment here letting us know!

Detected Issues (click me)
Check Known Missing Docstrings...........................................Failed
- hook id: invoke
- duration: 1.15s
- exit code: 1

/home/runner/.cache/pre-commit/repo6oivms_d/py_env-python3/lib/python3.9/site-packages/_distutils_hack/init.py:30: UserWarning: Setuptools is replacing distutils.
warnings.warn("Setuptools is replacing distutils.")
The function 'get_zabbix_id_mapper' on 'salt/modules/zabbix.py' does not have a 'CLI Example:' in it's docstring
The function 'substitute_params' on 'salt/modules/zabbix.py' does not have a 'CLI Example:' in it's docstring
The function 'compare_params' on 'salt/modules/zabbix.py' does not have a 'CLI Example:' in it's docstring
The function 'get_object_id_by_params' on 'salt/modules/zabbix.py' does not have a 'CLI Example:' in it's docstring
Found 4 errors


Thanks again!

@garethgreenaway
Copy link
Contributor

@t0fik all good. Just making sure the PR addresses the right issues, if this doesn't cover all the other issues but future PRs might that's great. The Zabbix modules are one collection of modules that we've considered moving out into a Salt extension separate from the main Salt code. This will allow it to be updated quicker than it has previously. It would live under a community controlled organization but wouldn't be actively maintained by the Salt core team. Would you be interested in being the owner of such an extension?

@garethgreenaway garethgreenaway merged commit 870e2d4 into saltstack:master May 13, 2022
@t0fik
Copy link
Contributor Author

t0fik commented Jul 15, 2022

@garethgreenaway Sorry for late response. I was loaded with other tasks. I think, that I'm not the best candidate to be the owner. I will contribute when I find any problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phosphorus v3005.0 Release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] zabbix_host.present groupid is missing
2 participants