-
Notifications
You must be signed in to change notification settings - Fork 733
Clarify when variables and defaults are automatically included in nested directories #3090
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
base: devel
Are you sure you want to change the base?
Conversation
…ted directories It's not immediately obvious when reading the existing paragraph when variables in nested directories are included or not. This attempts to disambiguate it by explicitly calling out that anything not under `main` will not be included automatically, and by providing an additional negative example that illustrate exactly this behaviour. Fixes ansible#2994.
Thanks for your Ansible docs contribution! We talk about Ansible documentation on Matrix at #docs:ansible.im if you ever want to join us and chat about the docs! We meet on Matrix every Tuesday. See the Ansible calendar for meeting details. We welcome additions to our weekly agenda items too. You can add the |
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.
I think the example helps. I had a few additional suggestions.
windows/ | ||
windows.yml | ||
In this example, no variables are included automatically. Instead, they should be included explicitly (e.g. using the ``vars_from`` in ``import_role`` statements). |
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.
In this example, no variables are included automatically. Instead, they should be included explicitly (e.g. using the ``vars_from`` in ``import_role`` statements). | |
In this example, no variables are included automatically. Instead, they should be included explicitly (for example, using the ``vars_from`` argument in an ``import_role`` task). |
windows/ | ||
windows.yml | ||
In this example, no variables are included automatically. Instead, they should be included explicitly (e.g. using the ``vars_from`` in ``import_role`` statements). |
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.
The reporter is using the roles
keyword, so I think we should add a note/warning that roles
and dependencies
cannot configure other entrypoints. (Also related: ansible/ansible#85876)
There's a note in the roles
section (below, which links to this section) that says:
vars and defaults can also match to a directory of the same name and Ansible will process all the files contained in that directory. See Role directory structure for more details.
I think it should say "a directory named 'main'", instead.
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.
Could you clarify that first part? Are you suggesting to add a note that explicitly calls out that for instance vars_from
in roles
won't work? Basically also fixing ansible/ansible#85876 in this PR?
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.
Are you suggesting to add a note that explicitly calls out that for instance vars_from in roles won't work?
Yes, I'm saying instead of just mentioning import_role
(again!) in this section related to the role entrypoints, the documentation should be more clear that roles
and dependencies
don't support loading vars/tasks/handlers from anything other than main.
Basically also fixing ansible/ansible#85876 in this PR?
Not really, I think we need to clarify the documentation on params, and the only two ways to use them now (roles
and dependencies
), and that generally only import_role
and include_role
receive new features and are recommended.
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.
Ack! I'll try and incorporate that in the coming days. Thanks for the feedback so far!
- Variables from ``vars/`` and ``defaults/`` are imported into play scope unless you disable it via the ``public`` option in ``import_role``/``include_role``. | ||
|
||
You can add other YAML files in some directories, but they won't be used by default. They can be included/imported directly or specified when using ``include_role/import_role``. |
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.
You can ignore or rephrase this suggestion, it's only tangentially related, but I had to read this line multiple times to understand it.
By 'other YAML files' I think it means non-main.yml
/main.yaml
/main
files, and by 'some directories' I think it means tasks
/handlers
/defaults
/vars
.
To include files from outside of tasks
/defaults
/vars
directories, include_tasks
/import_tasks
/include_vars
must be used, not include_role
/import_role
. Handlers just can't be loaded from other directories since there's no mechanism (though an include_tasks
handler in a standard location can run tasks from an arbitrary directory).
You can add other YAML files in some directories, but they won't be used by default. They can be included/imported directly or specified when using ``include_role/import_role``. | |
You can add YAML files named something other than ``main.yml`` in ``tasks/``, ``handlers/``, ``defaults/``, and ``vars/``, but they won't be used by default. They can be specified when using ``include_role``/``import_role``. Note that ``include_role`` and ``import_role`` do not support loading from other directories. You can include tasks/variables from arbitrary locations using ``include_tasks``/``import_tasks``/``include_vars``. |
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.
Yeah sorry I didn't necessarily want to refactor this, there were just a bunch of trailing spaces and I figured I might as well remove them. I can definitely rephrase this though if this is something you think would improve this page 🙂
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.
I definitely think rephrasing it would improve this page. I understand how the feature works, yet I had to read this line multiple times to understand what it meant. Your change modifies this same section, and is covering the same topic. Is there something you object to about my suggested change?
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.
Not at all, on the contrary. I just wanted to explain the context of this little whitespace change. I'll definitely incorporate your suggestion 👍
It's not immediately obvious when reading the existing paragraph when variables in nested directories are included or not. This attempts to disambiguate it by explicitly calling out that anything not under
main
will not be included automatically, and by providing an additional negative example that illustrate exactly this behaviour.Fixes #2994.