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

Support recursive suboptions schema #37206

Merged
merged 2 commits into from
Mar 9, 2018

Conversation

sivel
Copy link
Member

@sivel sivel commented Mar 8, 2018

SUMMARY

argument_spec allows "recursive" options, the documentation should allow recursive suboptions.

This requires voluptuous>=0.11.0

See #37196

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

test/sanity/validate-modules/schema.py

ANSIBLE VERSION
devel
ADDITIONAL INFORMATION

Before:

ansible/lib/ansible/modules/network/fortios/fortios_webfilter.py:0:0: E305 DOCUMENTATION.options.webfilter_url.suboptions.entries.suboptions: extra keys not allowed @ data['options']['webfilter_url']['suboptions']['entries']['suboptions']. Got {'id': {'description': ['Id of URL.'], 'required': True, 'type': 'integer'}, 'url': {'description': ['URL to be filtered.'], 'required': True}, 'type': {'description': ['Filter type (simple, regex, or wildcard).'], 'required': True, 'choices': ['simple', 'regex', 'wildcard']}, 'action': {'description': ['Action to take for URL filter matches.'], 'required': True, 'choices': ['exempt', 'block', 'allow', 'monitor']}, 'status': {'description': ['Enable/disable this URL filter.'], 'required': Tru...
ansible/lib/ansible/modules/network/fortios/fortios_webfilter.py:0:0: E305 DOCUMENTATION.options.webfilter_content.suboptions.entries.suboptions: extra keys not allowed @ data['options']['webfilter_content']['suboptions']['entries']['suboptions']. Got {'name': {'description': ['Banned word.'], 'required': True}, 'pattern-type': {'description': [{'Banned word pattern type': 'wildcard pattern or Perl regular expression.'}], 'required': True, 'choices': ['wildcard', 'regexp']}, 'status': {'description': ['Enable/disable banned word.'], 'required': True, 'choices': ['enable', 'disable']}, 'lang': {'description': ['Language of banned word.'], 'required': True, 'choices': ['western', 'simch', 'trach', 'japanese', 'korean', 'french', 'thai', 'spa...

After:

ansible/lib/ansible/modules/network/fortios/fortios_webfilter.py:0:0: E305 DOCUMENTATION.options.webfilter_url.suboptions.entries.suboptions.id.type: not a valid value for dictionary value @ data['options']['webfilter_url']['suboptions']['entries']['suboptions']['id']['type']. Got 'integer'
ansible/lib/ansible/modules/network/fortios/fortios_webfilter.py:0:0: E305 DOCUMENTATION.options.webfilter_content.suboptions.entries.suboptions.pattern-type.description.0: expected str @ data['options']['webfilter_content']['suboptions']['entries']['suboptions']['pattern-type']['description'][0]. Got {'Banned word pattern type': 'wildcard pattern or Perl regular expression.'}

@sivel sivel requested review from mattclay and gundalow March 8, 2018 17:49
@ansibot
Copy link
Contributor

ansibot commented Mar 8, 2018

@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels Mar 8, 2018
@mattclay mattclay removed the needs_triage Needs a first human triage before being processed. label Mar 8, 2018
Copy link
Contributor

@gundalow gundalow left a comment

Choose a reason for hiding this comment

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

Oh, that was easy :)

@@ -171,6 +173,6 @@ def metadata_1_1_schema(deprecated):
# 1) Don't allow empty options for choices, aliases, etc
# 2) If type: bool ensure choices isn't set - perhaps use Exclusive
# 3) both version_added should be quoted floats
# 4) Use Recursive Schema: https://github.com/alecthomas/voluptuous/issues/128 though don't allow two layers
# 4) Use Recursive Schema via `Self`, see `suboption_schema` above
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this line now as this is tracking other todo's

Copy link
Member

@mattclay mattclay left a comment

Choose a reason for hiding this comment

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

Please add the version constraint for voluptuous to test/runner/requirements/constraints.txt.

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 8, 2018
@sivel
Copy link
Member Author

sivel commented Mar 8, 2018

Please add the version constraint for voluptuous to test/runner/requirements/constraints.txt.

Done.

@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 8, 2018
@sivel sivel merged commit 8980d27 into ansible:devel Mar 9, 2018
privateip added a commit that referenced this pull request Mar 13, 2018
@dagwieers dagwieers added the fortios Fortios community label Feb 22, 2019
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug. fortios Fortios community support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants