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.

Sign up for GitHub

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

Jump to bottom

lookup(first_found) with errors=ignore returns empty string #56775

Closed
philfry opened this issue May 22, 2019 · 1 comment · Fixed by #57038
Closed

lookup(first_found) with errors=ignore returns empty string #56775

philfry opened this issue May 22, 2019 · 1 comment · Fixed by #57038
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. support:community This issue/PR relates to code supported by the Ansible community.

Comments

@philfry
Copy link
Contributor

philfry commented May 22, 2019

SUMMARY
lookup("first_found", whatever, errors="ignore")

returns an empty list instead of either None or an empty list, breaking the skip compatibility.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

first_found

ANSIBLE VERSION
ansible 2.9.0.dev0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/phil/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/phil/projects/3rdparty/ansible/lib/ansible
  executable location = /home/phil/projects/3rdparty/ansible/bin/ansible
  python version = 2.7.16 (default, Apr 30 2019, 15:54:43) [GCC 9.0.1 20190312 (Red Hat 9.0.1-0.10)]
CONFIGURATION

n/a

OS / ENVIRONMENT

n/a

STEPS TO REPRODUCE
---
- hosts: localhost
  gather_facts: no
  tasks:
    - debug:
        msg: '{{q("first_found", dict(files=["a.yml", "b.yml", "c.yml"], skip=true))}}'
    - debug:
        msg: '{{q("first_found", dict(files=["a.yml", "b.yml", "c.yml"]), errors="ignore")}}'
EXPECTED RESULTS
ok: [localhost] => {
    "msg": []
}

for both tasks

ACTUAL RESULTS
PLAY [localhost] ***************************************************************************

TASK [debug] *******************************************************************************
[DEPRECATION WARNING]: Use errors="ignore" instead of skip. This feature will be removed in
 version 2.12. Deprecation warnings can be disabled by setting deprecation_warnings=False 
in ansible.cfg.
ok: [localhost] => {
    "msg": []
}

TASK [debug] *******************************************************************************
ok: [localhost] => {
    "msg": ""
}

PLAY RECAP *********************************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0
@philfry philfry mentioned this issue May 22, 2019
Closed
@ansibot
Copy link
Contributor

ansibot commented May 22, 2019

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. labels May 22, 2019
@ianw ianw mentioned this issue May 28, 2019
Merged
ianw added a commit to ianw/ansible that referenced this issue May 28, 2019
@ianw
Make query with errors='ignore' return a blank list
a79208d 
The jinja2 query() function (or lookup with wantslist=True, which is
the same thing) should always return a list.

However, if you combine a query with errors='ignore' and take the
error path, the current code returns a None value.  This is important
in a case such as

 - name: Conditional include file
   import_tasks: '{{ item }}'
   vars:
     params:
       files:
         - path/file1.yaml
         - path/file2.yaml
   loop: "{{ q('first_found', params, errors='ignore') }}"

If neither file1.yaml or file2.yaml exist, this should do nothing by
returning an empty list to the loop.  Currently if you run the above
task you'll get a rather unhelpful:

 Invalid data passed to 'loop', it requires a list, got this instead: .

This change ensures that when a query ignores an error, it returns a
empty list.  The errors='ignore' case is tested in several variants
with first_found.  The extant (but deprecated) "skip: True" for
first_found doesn't seem to be explicitly tested; a test is added here
to avoid regressions before removal in 2.12.

This fixes a regression you'll hit if you follow the suggestion in the
deprecation message included with
e17a2b5 to use errors=ignore over
"skip: True" for first_found.  This change adds an example that points
out the query/lookup difference and also fixes the error message to
not mention the now deprecated "skip: True".

Closes ansible#56775
StephenSorriaux pushed a commit to StephenSorriaux/ansible that referenced this issue May 29, 2019
@ianw @bcoca
Make query with errors='ignore' return a blank list (ansible#57038)
39945b8 
The jinja2 query() function (or lookup with wantslist=True, which is
the same thing) should always return a list.

However, if you combine a query with errors='ignore' and take the
error path, the current code returns a None value.  This is important
in a case such as

 - name: Conditional include file
   import_tasks: '{{ item }}'
   vars:
     params:
       files:
         - path/file1.yaml
         - path/file2.yaml
   loop: "{{ q('first_found', params, errors='ignore') }}"

If neither file1.yaml or file2.yaml exist, this should do nothing by
returning an empty list to the loop.  Currently if you run the above
task you'll get a rather unhelpful:

 Invalid data passed to 'loop', it requires a list, got this instead: .

This change ensures that when a query ignores an error, it returns a
empty list.  The errors='ignore' case is tested in several variants
with first_found.  The extant (but deprecated) "skip: True" for
first_found doesn't seem to be explicitly tested; a test is added here
to avoid regressions before removal in 2.12.

This fixes a regression you'll hit if you follow the suggestion in the
deprecation message included with
e17a2b5 to use errors=ignore over
"skip: True" for first_found.  This change adds an example that points
out the query/lookup difference and also fixes the error message to
not mention the now deprecated "skip: True".

Closes ansible#56775
@mkrizek mkrizek removed needs_triage Needs a first human triage before being processed. labels May 30, 2019
@Normo Normo mentioned this issue Jun 6, 2019
Closed
@ansible ansible locked and limited conversation to collaborators Aug 5, 2019
bcoca pushed a commit to bcoca/ansible that referenced this issue Feb 21, 2020
@ianw @bcoca
Make query with errors='ignore' return a blank list (ansible#57038)
87658d9 
The jinja2 query() function (or lookup with wantslist=True, which is
the same thing) should always return a list.

However, if you combine a query with errors='ignore' and take the
error path, the current code returns a None value.  This is important
in a case such as

 - name: Conditional include file
   import_tasks: '{{ item }}'
   vars:
     params:
       files:
         - path/file1.yaml
         - path/file2.yaml
   loop: "{{ q('first_found', params, errors='ignore') }}"

If neither file1.yaml or file2.yaml exist, this should do nothing by
returning an empty list to the loop.  Currently if you run the above
task you'll get a rather unhelpful:

 Invalid data passed to 'loop', it requires a list, got this instead: .

This change ensures that when a query ignores an error, it returns a
empty list.  The errors='ignore' case is tested in several variants
with first_found.  The extant (but deprecated) "skip: True" for
first_found doesn't seem to be explicitly tested; a test is added here
to avoid regressions before removal in 2.12.

This fixes a regression you'll hit if you follow the suggestion in the
deprecation message included with
e17a2b5 to use errors=ignore over
"skip: True" for first_found.  This change adds an example that points
out the query/lookup difference and also fixes the error message to
not mention the now deprecated "skip: True".

Closes ansible#56775
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make query with errors='ignore' return a blank list ianw/ansible
3 participants
@mkrizek @philfry @ansibot