-
Notifications
You must be signed in to change notification settings - Fork 488
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
Update module default groups documentation #1893
base: devel
Are you sure you want to change the base?
Conversation
Co-authored-by: Felix Fontein <felix@fontein.de>
Thanks for reviewing @felixfontein! |
One general thing I'm wondering about this documentation is: why is part of the syntax for collection development documented here? There's also a separate set of documentation for this feature in dev_guide/developing_collections_structure.rst, which also mentions the special - *action_groups*
A mapping of groups and the list of action plugin and module names they contain. They may also have a special 'metadata' dictionary in the list, which can be used to include actions from other groups.
.. code:: yaml
action_groups:
groupname:
# The special metadata dictionary. All action/module names should be strings.
- metadata:
extend_group:
- another.collection.groupname
- another_group
- my_action
another_group:
- my_module
- another.collection.another_module Maybe the meta/runtime part of the documentation here should be deleted, and the other documentation should be referenced instead? (There's also essentially no validation of |
I added both in 74039. Linking to the other would be better, I think I just wanted to make sure it was mentioned both places. Adding a good sanity test seems like it should be a prerequisite for documenting this as no longer preview. Fwiw, I made the runtime behavior for action_groups very forgiving, it's at most a warning, and there's a toggle to ignore invalid action_groups: https://github.com/ansible/ansible/blob/devel/lib/ansible/playbook/base.py#L33-L69. |
Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com>
Yes, this is a little bit weird. I should say that this file should document how to use module defaults, but not how to implement them in a collection. |
I agree, though right now users often have to consult the implementation to find out which module default groups are available and which actions they cover. (You can document this with attributes for an individual module/action, but not every collection is doing that. And even that doesn't give you a list of all actions in an action group either, it only tells you for a specific action whether its in there. So being able to look at meta/runtime.yml right now is a necessary skill for users of action groups...) |
I created a better validation test in ansible/ansible#83965. |
@felixfontein @s-hertel I'm unsure why this PR hasn't been merged yet. Is there anything I could do to improve it? Or does it look OK to you now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from my side. If this is also OK from Core's side, let's merge this.
Fixes #1759
It looks like default groups / action groups are stable. I think we should update the documentation accordingly, since it still says it's a preview feature.
I've also learned that that there is some magic happening for some special groups for reasons of downwards compatibility or whatever:
https://github.com/ansible/ansible/blob/b5ae8a382baca04442ad82e654ab6e35379844c3/lib/ansible/config/ansible_builtin_runtime.yml#L9685
I think we should document this also.