ansible-documentation
ansible-documentation copied to clipboard
ansible
check_mode attribute description should reflect their support status
Right now the attribute description for check_mode always says
Can run in check_mode and return changed status prediction without modifying target
Even if the module explicitly doesn't support check mode (support none) like in the uri example. I can see this being confusing for people reading the docs who might skip over the Support side and read Description thinking this does support check mode when it does not.
I think the description for entries where check mode isn't supported should be updated to actually say something like "this module does not support check mode" in the field.
Related: https://github.com/ansible/ansible/pull/82427
This should be in the antsibull-docs repo. I can't 'transfer' across github orgs so closing this in favor of https://github.com/ansible-community/antsibull-docs/issues/235
The question should be discussed either here or in ansible/ansible. antsibull-docs has nothing to do with that text.
Right now attributes work as follows: every module/plugin has provide a list of attributes, and every attribute must have a support value and a description and optionally details (and for some special attribute names, some more information). So in the end, every module is responsible for providing a fitting text.
ansible-core provides the action_common_attributes docs fragment, which provides a standard text for check_mode (https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/doc_fragments/action_common_attributes.py#L13) that is used by all modules in ansible-core, no matter whether they add support: none, support: full, or support: partial.
Other collections either also use that docs fragment, or (especially if they support older versions of ansible-core which doesn't have that fragment) often have a copy or modified copy of that docs fragment (for example https://github.com/ansible-collections/community.general/blob/main/plugins/doc_fragments/attributes.py in community.general).
So if we want to make the description depend on the support value, the module/plugin authors have to provide that description. To avoid having to repeat the description in every single module/plugin. The answer here should be obvious: docs fragments. If your module supports check mode, you can use the docs fragments ansible.builtin.action_common_attributes and ansible.builtin.action_common_attributes.check_mode_full. If your module does not support check mode, you can use the docs fragments ansible.builtin.action_common_attributes and ansible.builtin.action_common_attributes.check_mode_none. (And the same for partial, where you have to add details to say what exactly you mean by 'partial'.) So basically support: none or support: full for check_mode in the module/plugin is replaced by another docs fragment that's included. (I don't think there's any other sensible way of implementing this.)
So the main thing left are two things:
- We need to decide whether the description should actually depend on the support value. My understanding is that the origina intention was that the description of the attribute should not depend on whether the module supports it or not, but should describe the attribute itself. The support value says whether this attribute is supposed by the module/plugin or not (with some gray area when
support: partial, for which alsodetailsexists). So we have to discuss whether we want to change away from that dogma. - If we decide that the description should depend on the level, we need to decide on the texts. (And also for which attributes we want to do this. Check mode is one of many attributes.)
So looking at the table, my personal opinion is no change is needed. The support is clear (and even color-coded). I don't find it confusing.
My concern is that the description could somewhat be interpreted as the current state rather than a description of the attribute. We could potentially just update the raw value for this module to give a better description to reflect the attribute value that is set but I opened this to see if we wanted something that is more generic for other plugins that also do not support check mode.
Personally I was never really that fond of the whole free form attribute setup and that there aren't any "known" ones outside of common conventions.
Would it work better as attribute | description | support|
As in would it make it clearer that the description is just what the attribute is, not whether it's supported so to speak?
We could also change the attribute's description so it's clearer that it describes the attribute in general, and not the attribute for this specific module/plugin. For example:
Full support means that the module can run in check_mode and return changed status prediction without modifying target.
Discussed in DaWGs and the general feel is to change the text as mentioned in https://github.com/ansible/ansible-documentation/issues/1006#issuecomment-1877807975. The current table renders better in smaller screens with the support field right after the attribute field
So based on this, I think I need to move/copy this to ansible/ansible repo, but want to check here before I do that. Does that make sense @felixfontein @jborean93 ?
If you want modifications to the layout, you need to create an issue in gh.com/ansible-community/antsibull-docs. If you only want to adjust the texts (which is what I understand), then moving to ansible/ansible is ok.
On the other hand, it might make sense to keep the issue here and create a new (more specific) one in ansible/ansible, so we have an issue to track updating the copies / similar fragments to the one from ansible-core in various collections.