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

vmware_host_snmp: Add code to support SNMP values #1103

Merged

Conversation

tejaswi-bachu
Copy link
Contributor

@tejaswi-bachu tejaswi-bachu commented Nov 10, 2021

Depends-On: #1102

SUMMARY

Added code to support setting SNMP syscontact and syslocation.

Fixes #1044

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

vmware_host_snmp.py
community.vmware.vmware_host_snmp_module.rst

ADDITIONAL INFORMATION

@tejaswi-bachu tejaswi-bachu changed the title Add code to support SNMP issue 1044 vmware_host_snmp: Add code to support SNMP issue 1044 Nov 10, 2021
Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a changelog fragment? Something like changelogs/fragments/1044-vmware_host_snmp.yml:

minor_changes:
  - vmware_host_snmp - implement setting syscontact and syslocation (https://github.com/ansible-collections/community.vmware/issues/1044).

And I think this might be worth a test case in tests/integration/targets/vmware_host_snmp/tasks/main.yml. Do you think you can do this? It should be mostly copy&paste, add setting syscontact / syslocation and assert that you get a change.

@mariolenz
Copy link
Collaborator

I'm not sure if a recheck would take into account the Depends-On: #1102 that I've added. So I'll try to close and re-open this PR.

@mariolenz mariolenz closed this Nov 10, 2021
@mariolenz mariolenz reopened this Nov 10, 2021
@goneri
Copy link
Member

goneri commented Nov 10, 2021

@mariolenz when you set a Depends-On:, you need to use the full path, e.g: https://github.com/ansible-collections/community.vmware/pull/1102, instead of #1102.

@mariolenz
Copy link
Collaborator

when you set a Depends-On:, you need to use the full path, e.g: https://github.com/ansible-collections/community.vmware/pull/1102, instead of #1102.

Sorry, I knew this but I tend to forget it. Maybe because

Depends-On: https://github.com/ansible-collections/community.vmware/pull/1102

looks exactly like

Depends-On: #1102

:-/

@mariolenz mariolenz closed this Nov 10, 2021
@mariolenz mariolenz reopened this Nov 10, 2021
@goneri
Copy link
Member

goneri commented Nov 10, 2021

:-/

No worry :-). I know it's confusing.

docs/community.vmware.vmware_host_snmp_module.rst Outdated Show resolved Hide resolved
plugins/modules/vmware_host_snmp.py Outdated Show resolved Hide resolved
plugins/modules/vmware_host_snmp.py Outdated Show resolved Hide resolved
plugins/modules/vmware_host_snmp.py Show resolved Hide resolved
plugins/modules/vmware_host_snmp.py Show resolved Hide resolved
plugins/modules/vmware_host_snmp.py Show resolved Hide resolved
@Akasurde Akasurde added this to the v.1.17.0 milestone Nov 11, 2021
@Akasurde Akasurde changed the title vmware_host_snmp: Add code to support SNMP issue 1044 vmware_host_snmp: Add code to support SNMP values Nov 11, 2021
@tejaswi-bachu
Copy link
Contributor Author

@mariolenz @Akasurde - Thanks for your feedback. I made the required changes. Please review.

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

What do you say @Akasurde?

goneri and others added 2 commits November 13, 2021 19:27
Co-authored-by: Abhijeet Kasurde <akasurde@redhat.com>
Co-authored-by: Abhijeet Kasurde <akasurde@redhat.com>
@goneri goneri added the gate label Nov 14, 2021
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Akasurde
Copy link
Member

recheck

@goneri
Copy link
Member

goneri commented Nov 16, 2021

recheck

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The duplicate version_added in lines 80 / 81 and 86 / 87 are a problem for our sanity checks.

plugins/modules/vmware_host_snmp.py Outdated Show resolved Hide resolved
plugins/modules/vmware_host_snmp.py Outdated Show resolved Hide resolved
@mariolenz
Copy link
Collaborator

recheck

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mariolenz mariolenz added the gate label Nov 17, 2021
@ansible-zuul ansible-zuul bot merged commit 3061ee7 into ansible-collections:main Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding support for SNMP syslocation and syscontact configuration
4 participants