kubernetes.core
kubernetes.core copied to clipboard
ansible-collections
Add valid type parameter to documentation for resource_definition
SUMMARY
Inside common.py, the resource_definition argument uses a custom type, list_dict_str.
Because of this, and the ansible-test bug Unable to pass validate-modules sanity check if using custom type for arg spec, I can't get the argument in the documentation fragment k8s_resource_options.py to pass the validate-modules test. I get the errors:
Run command: /usr/bin/python3.6 /root/ansible/test/lib/ansible_test/_data/sanity/validate-modules/validate-modules --format json --arg-spec plugins/modules/k8s.py plugins/modules/k8s_auth.py plugins/modules/k8s_ ...
ERROR: Found 2 validate-modules issue(s) which need to be resolved:
ERROR: plugins/modules/k8s.py:0:0: parameter-type-not-in-doc: Argument 'resource_definition' in argument_spec defines type as <function list_dict_str at 0x7f7570d09378> but documentation doesn't define type
ERROR: plugins/modules/k8s_scale.py:0:0: parameter-type-not-in-doc: Argument 'resource_definition' in argument_spec defines type as <function list_dict_str at 0x7f7570d09378> but documentation doesn't define type
I have added the k8s and k8s_scale modules to the sanity check ignore.txt file for now for parameter-type-not-in-doc, but for this issue, I'd like to hopefully remove those (and have no more ignore.txt content).
ISSUE TYPE
- Feature Idea
initially I used the custom type because the from_yaml filter did not support multidocument yaml, so the only way to parse it was either forcing the user to do it by manually splitting the file (gross), or loading it in as a string ourselves. It also supports lists because a multidocument yaml is conceptually just a list of resources, and we added the from_yaml_all filter that returns a list. Now that we have the from_yaml_all filter though it's definitely much less necessary to have the multi-document parsing supported via type hacks, and we could totally just tell users to use the from_yaml_all filter and loop via a normal Ansible loop (which is probably better anyway, since that will provide more granular output on the Ansible side)
I'd be happy to go that way, especially since this new collection gives us an opportunity to make little changes which may affect some edge use cases that we don't want to support anyways—if we make that change here, people using Ansible 2.9 still won't be affected, and when they move to this collection's content (either before or after 2.10's release) they'll likely need to tweak some things anyways, so switching to from_yaml_all if they aren't already using it would be one of those things.
At this point, if we want to incorporate this change (as @fabianvf suggested), I would propose doing that in 2.x since it could be a breaking change.
