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

Tidy up validate-modules ignores for modules: net_tools/nios #1598

Merged
merged 29 commits into from
Jan 11, 2021
Merged

Tidy up validate-modules ignores for modules: net_tools/nios #1598

merged 29 commits into from
Jan 11, 2021

Conversation

russoz
Copy link
Collaborator

@russoz russoz commented Jan 6, 2021

SUMMARY

Tidy up on sanity tests' ignore files, removing lines with validation-modules for these modules.

The focus was to pass the tests so no code was changed other than adjusting argument_spec for missing types or inconsistencies with the docs. The priority has been not to change any functionality, but to ensure that code and docs are synchronized.

A total of 238 lines were removed (among 2.9, 2.10 and 2.11).

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

plugins/modules/net_tools/nios/nios_nsgroup.py
plugins/modules/net_tools/nios/nios_zone.py
plugins/modules/net_tools/nios/nios_txt_record.py
plugins/modules/net_tools/nios/nios_srv_record.py
plugins/modules/net_tools/nios/nios_ptr_record.py
plugins/modules/net_tools/nios/nios_network_view.py
plugins/modules/net_tools/nios/nios_network.py
plugins/modules/net_tools/nios/nios_naptr_record.py
plugins/modules/net_tools/nios/nios_mx_record.py
plugins/modules/net_tools/nios/nios_member.py
plugins/modules/net_tools/nios/nios_host_record.py
plugins/modules/net_tools/nios/nios_fixed_address.py
plugins/modules/net_tools/nios/nios_dns_view.py
plugins/modules/net_tools/nios/nios_cname_record.py
plugins/modules/net_tools/nios/nios_aaaa_record.py
plugins/modules/net_tools/nios/nios_a_record.py

ADDITIONAL INFORMATION

There was a bit of coding there, so the boundary between docs and bugfix PR is a little fuzzy. Please let me know if a bugfix changelog should be added.

russoz added 20 commits January 6, 2021 21:25
- ``http_pool_connections``
- ``http_pool_maxsize``
- ``silent_ssl_warnings``
@ansibullbot ansibullbot added the tests tests label Jan 6, 2021
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

You definitely need a changelog fragment for the code changes.

plugins/modules/net_tools/nios/nios_nsgroup.py Outdated Show resolved Hide resolved
plugins/modules/net_tools/nios/nios_nsgroup.py Outdated Show resolved Hide resolved
plugins/modules/net_tools/nios/nios_nsgroup.py Outdated Show resolved Hide resolved
plugins/modules/net_tools/nios/nios_nsgroup.py Outdated Show resolved Hide resolved
plugins/modules/net_tools/nios/nios_nsgroup.py Outdated Show resolved Hide resolved
plugins/modules/net_tools/nios/nios_nsgroup.py Outdated Show resolved Hide resolved
@ansibullbot
Copy link
Collaborator

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

plugins/modules/net_tools/nios/nios_member.py:0:0: doc-default-does-not-match-spec: Argument 'enable_ha' in argument_spec defines default as (False) but documentation defines default as (None)
plugins/modules/net_tools/nios/nios_member.py:0:0: doc-default-does-not-match-spec: Argument 'lan2_enabled' in argument_spec defines default as (False) but documentation defines default as (None)

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

plugins/modules/net_tools/nios/nios_member.py:0:0: doc-default-does-not-match-spec: Argument 'enable_ha' in argument_spec defines default as (False) but documentation defines default as (None)
plugins/modules/net_tools/nios/nios_member.py:0:0: doc-default-does-not-match-spec: Argument 'lan2_enabled' in argument_spec defines default as (False) but documentation defines default as (None)

click here for bot help

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Jan 6, 2021
russoz and others added 6 commits January 7, 2021 11:41
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Jan 6, 2021
@russoz
Copy link
Collaborator Author

russoz commented Jan 7, 2021

I was looking forward to hear from the module maintainers on this one.

@felixfontein
Copy link
Collaborator

@russoz I have some doubts that there are active maintainers. Also, in theory, these modules should be removed here eventually and be replaced with the ones in https://github.com/infobloxopen/infoblox-ansible, but right now I'm not sure when that will happen. It does not look like anything will happen in the next weeks, i.e. it won't happen for Ansible 3.0.0 / community.general 2.0.0.

@russoz
Copy link
Collaborator Author

russoz commented Jan 11, 2021

@felixfontein in that case, given #1598 (comment) and given the fact these modules are passing both unit and integration tests, may we merge this PR?

BTW, I wouldn't mind creating another PR to add the deprecations - for 3.0.0? - to these guys. They would go along with the clous/ovirt ones.

@felixfontein
Copy link
Collaborator

@russoz we do not have integration tests for the NIOS modules that run in CI. That's why I'm hesistant of merging this.

About deprecating these: we should not deprecate them. We keep them until that other collection is included in Ansible (whenever that happens), and then remove the modules in the next major version (and add redirects).

Co-authored-by: Felix Fontein <felix@fontein.de>
@felixfontein
Copy link
Collaborator

The tests should no longer fail thanks to #1615.

@felixfontein felixfontein merged commit 6c7f8f9 into ansible-collections:main Jan 11, 2021
@felixfontein
Copy link
Collaborator

@russoz thanks a lot for improving the nios modules, and reducing sanity ignores! :)

@russoz russoz deleted the valmod_nios branch January 12, 2021 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review docs_fragments docs_fragments plugin (shared docs) docs module_utils module_utils module module net_tools plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants