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

Replace ansible.builtin.pause with ansible.builtin.wait_for #29

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

jhohwieler
Copy link

This fixes an issue when running with strategy free:

ERROR! The 'ansible.builtin.pause' module bypasses the host loop, which
is currently not supported in the free strategy and would instead
execute for every host in the inventory list.

Furthermore, the documentation for ansible.builtin.pause states: To pause/wait/sleep per host, use the ansible.builtin.wait_for module.

See https://docs.ansible.com/ansible/latest/collections/ansible/builtin/pause_module.html

This fixes an issue when running with strategy free:
> ERROR! The 'ansible.builtin.pause' module bypasses the host loop, which
> is currently not supported in the free strategy and would instead
> execute for every host in the inventory list.

Furthermore, the documentation for ansible.builtin.pause states:
To pause/wait/sleep per host, use the ansible.builtin.wait_for module.

See https://docs.ansible.com/ansible/latest/collections/ansible/builtin/pause_module.html
@Wabri Wabri added the enhancement New feature or request label Aug 20, 2024
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, someone should add a comment here to understand the need for this 10s pause.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git blame said that the first introduction of this was made by @berndfinger, do you remember something about this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Wabri I am shown in git blame because I, if I remember correctly, created the initial version by transferring it from another repo.

@rainerleber - I see that you changed some lines in this file after the initial version. Do you know more about why the pause module was used here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i looks like this wait in the main.yaml could be removed, maybe it's an historic entry to work as expected before we included the modules there was a script to discover the facts

@jhohwieler
Copy link
Author

Is there a timeline to get this enhancement change into mainline?

Copy link
Collaborator

@rainerleber rainerleber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jhohwieler
Copy link
Author

Basically nothing happened here since my last request. When will you integrate the 3 approved pull request into Main?

@kksat
Copy link

kksat commented Nov 12, 2024

@jhohwieler you are welcome to use sap.sap_operations collection
https://github.com/redhat-sap/rh_operations
https://galaxy.ansible.com/ui/repo/published/sap/sap_operations/

There we release version almost every week.

If you are missing something in mentioned collection let me know, please.

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

Successfully merging this pull request may close these issues.

5 participants