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

Prefer ansible_facts namespace in examples #53595

Open
wants to merge 1 commit into
base: devel
from

Conversation

@netzvieh
Copy link
Contributor

netzvieh commented Mar 9, 2019

SUMMARY

We should prefer the use of ansible_facts in the examples. Also remove one mention of dot vs. array notation in faq.rst since it has it's own explanation a few lines down.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 9, 2019

@netzvieh, just so you are aware we have a dedicated Working Group for network.
You can find other people interested in this in #ansible-network on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@caphrim007

This comment has been minimized.

Copy link
Contributor

caphrim007 commented Mar 10, 2019

I have no issue with this for the BIG-IP modules. @wojtek0806 to backport

@ansibot ansibot removed the needs_triage label Mar 10, 2019

@bcoca

bcoca approved these changes Mar 11, 2019

@dagwieers
Copy link
Member

dagwieers left a comment

I am more in favor of the dot-notation for many examples as it reads better than additional brackets.

Show resolved Hide resolved docs/docsite/rst/user_guide/playbooks_filters.rst Outdated

@ansibot ansibot added needs_revision and removed core_review labels Mar 11, 2019

@netzvieh netzvieh force-pushed the netzvieh:use-ansible_facts-format-in-examples branch from c0f86de to d679a0e Mar 11, 2019

@felixfontein

This comment has been minimized.

Copy link
Contributor

felixfontein commented Mar 12, 2019

We should prefer the use of ansible_facts in the examples.

Out of curiosity: why should be prefer that?

@dagwieers

This comment has been minimized.

Copy link
Member

dagwieers commented Mar 12, 2019

My personal view on this is that we should move to a built-in facts variable so that Ansible facts become available through facts.hostname. And only advertise this once we are at the end of this evolutionary-path-of-improvement. But there's no consensus where exactly we are going, so it makes it impossible to come to a concise, practical and well-designed language/syntax IMO.

@netzvieh

This comment has been minimized.

Copy link
Contributor Author

netzvieh commented Mar 12, 2019

@felixfontein I'm basically following the Ansible 2.5 Porting Guide here.

Ansible facts, which have historically been written to names like ansible_* in the main facts namespace, have been placed in their own new namespace, ansible_facts.* For example, the fact ansible_distribution is now best queried through the variable structure ansible_facts.distribution.

A new configuration variable, inject_facts_as_vars, has been added to ansible.cfg. Its default setting, ‘True’, keeps the 2.4 behavior of facts variables being set in the old ansible_* locations (while also writing them to the new namespace). This variable is expected to be set to ‘False’ in a future release. When inject_facts_as_vars is set to False, you must refer to ansible_facts through the new ansible_facts.* namespace.

@felixfontein

This comment has been minimized.

Copy link
Contributor

felixfontein commented Mar 12, 2019

Ok, that makes sense. But why use the array notation? It's pretty ugly, and also the changelog you refer to doesn't use it.

@bcoca

This comment has been minimized.

Copy link
Member

bcoca commented Mar 12, 2019

The array notation also avoids several issues:
for example mydict.keys (returns all keys in a dictionary) is not the same as mydict['keys'] (returns value assigne to 'keys' key) as the dot notation is implemented by Jinja via a fallback mechanism to access the dictionaries, but if you use a key with the same name as an existing method, you will invoke the method and NOT the key , returning unexpected results.

Another issue is that dot notation requires 'valid python identifiers' mydict.this-is-invalid will fail, while mydict['this-isnot-invalid'] will work since 'keys' have different requirements than 'attributes'.

@netzvieh

This comment has been minimized.

Copy link
Contributor Author

netzvieh commented Mar 15, 2019

ready_for_review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.