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

False positive - no-handler: Tasks that run when changed should likely be handlers. #3655

Closed
ReenigneArcher opened this issue Aug 11, 2023 · 2 comments · Fixed by #3702
Closed
Assignees
Labels

Comments

@ReenigneArcher
Copy link
Contributor

Summary

Ansible lint complains of no-handler: Tasks that run when changed should likely be handlers. when it is already a handler.

Issue Type
  • Bug Report
OS / ENVIRONMENT
ansible-lint --version

Using ansible/[email protected]

Unable to use new action at this time due to #3648

  • ansible installation method: one of source, pip, OS package
  • ansible-lint installation method: one of source, pip, OS package
    GitHub action.
STEPS TO REPRODUCE
- name: Test
  hosts:
    - test
  gather_facts: false
  connection: local

  tasks:
    - name: Get info
      delegate_to: localhost
      register: collected_info
      somemodule.that.collects.info:

    - name: Do something
      delegate_to: localhost
      loop: "{{ collected_info['some_list'] }}"
      loop_control:
        label: "{{ item.name }}"
      notify:
        - Debug
      register: _something_done
      somemodule.that.may.change.something:

  handlers:
    - name: Debug
      loop: "{{ _something_done.results }}"
      loop_control:
        label: "{{ item.item.name }}"
      when: item.changed
      ansible.builtin.msg:
        msg: "{{ item.item.name }} changed"
Desired Behavior

Don't warn that something should be a handler that already is a handler.

Possible security bugs should be reported via email to [email protected]

Actual Behavior

Please give some details of what is happening.
Include a minimum complete verifiable example with:

  • minimized playbook to reproduce the error
  • the output of running ansible-lint including the command line used
  • if you're getting a stack trace, also the output of
    ansible-playbook --syntax-check playbook

@ReenigneArcher ReenigneArcher added bug new Triage required labels Aug 11, 2023
@ajakk
Copy link

ajakk commented Aug 20, 2023

I'm hitting this too: https://github.com/ajakk/ansible/actions/runs/5919432937

The check isn't really wrong, but a more intelligent check might be taught that var.changed is a fine thing to check in handlers when looping, or that handlers shouldn't be treated as tasks for the purposes of this check, or both. Also reproduced from git at commit 566b036.

ajakk added a commit to ajakk/ansible that referenced this issue Aug 20, 2023
@ssbarnea ssbarnea self-assigned this Aug 23, 2023
ssbarnea added a commit that referenced this issue Aug 31, 2023
@ssbarnea ssbarnea removed the new Triage required label Sep 4, 2023
@ajakk
Copy link

ajakk commented Sep 9, 2023

This fix is incomplete in my case, where the handlers are actually in a separate tasks file. The new Task.is_handler seems to return false in this case.

Adding a pdb invocation in no_handler.py:

diff --git a/src/ansiblelint/rules/no_handler.py b/src/ansiblelint/rules/no_handler.py
index 165bbf43..8ce4a4a9 100644
--- a/src/ansiblelint/rules/no_handler.py
+++ b/src/ansiblelint/rules/no_handler.py
@@ -69,6 +69,7 @@ class UseHandlerRatherThanWhenChangedRule(AnsibleLintRule):
         task: Task,
         file: Lintable | None = None,
     ) -> bool | str:
+        import pdb; pdb.set_trace()
         if task["__ansible_action_type__"] != "task" or task.is_handler():
             return False
(Pdb) task
Task('Unmount filesystems' [.[0]])
(Pdb) task.position
'.[0]'
(Pdb) task.is_handler()
False

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants