zabbix_action - allowing string for esc_period

[rockaut]

SUMMARY

Fixes #51345

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

zabbix_action

ADDITIONAL INFORMATION

In my opinion a specific check for the value type regarding the Zabbix version isn't needed. If one doesn't read the readme and is using a string for <= 3.2 the API request would fail and an error is raised.

If I should add one please comment.

[ansibot]

cc @D3DeFi @K-dot @rubentsirunyan @sky-joker
click here for bot help

[ansibot]

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

test/sanity/ignore.txt:1970:1: A100: Ignoring 'doc-default-incompatible-type' on 'lib/ansible/modules/monitoring/zabbix/zabbix_action.py' is unnecessary

click here for bot help

[ansibot]

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

test/sanity/ignore.txt:1970:1: A100: Ignoring 'doc-default-incompatible-type' on 'lib/ansible/modules/monitoring/zabbix/zabbix_action.py' is unnecessary

click here for bot help

[rockaut]

Can someone please tell me what this CI error means? Don't get it.

[D3DeFi]

Can someone please tell me what this CI error means? Don't get it.

delete line 1970 from test/sanity/ignore.txt and you should be good to go. It seems that by introducing this patch, you have fixed the original reason, why was sanity test doc-default-incompatible-type skipped for this module. With your patch, this sanity test should no longer be ignored, thus shippable is complaining (new modules and PRs are required to match ansible sanity standards)

Edit: reason seems to be that esc_period has implicit default type: str in module documentation when it should have been explicit type: int before this patch

[ansibot]

cc @mattclay @tremble
click here for bot help

[D3DeFi]

Works for me with ZBX > 3.4

-label needs_triage
shipit

[rockaut]

Thank you @D3DeFi ... as a warning: there will be some more PR coming in the next weeks as I proceed to implement the different modules ;-)

[D3DeFi]

Thank you @D3DeFi ... as a warning: there will be some more PR coming in the next weeks as I proceed to implement the different modules ;-)

Thank you for being so active :)

[sky-joker]

Hi @rockaut

Could you please rebase and resolve the conflicts?

[rockaut]

Could you please rebase and resolve the conflicts?

@sky-joker can you give me a hint here? I've remove the one line because of the conversation with @D3DeFi on the sanity CI checks. As I understand the conflict it tells me I'm not allowed to remove this line but then the CI fails because it wants an int which is now a str because this would fix the "bug".

[D3DeFi]

Could you please rebase and resolve the conflicts?

@sky-joker can you give me a hint here? I've remove the one line because of the conversation with @D3DeFi on the sanity CI checks. As I understand the conflict it tells me I'm not allowed to remove this line but then the CI fails because it wants an int which is now a str because this would fix the "bug".

here ya go
https://docs.ansible.com/ansible/latest/dev_guide/developing_rebasing.html

edit: nothing wrong on your part. This is very common with frequently modified resources (as is the case with ignore.txt for sanity testing). You just got caught in the middle of never ending changes to upstream. This wouldn't be a problem if your PR had been merged within a minutes of you opening it, but that is unfortunately not always possible. Sorry and thanks for understanding :)

[D3DeFi]

cc @Akasurde (due to ignore.txt == support:core). Can this get merged please? :)
shipit

[sky-joker]

Thank you @rockaut for resolved conflicts!

shipit