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

Fix: documentation for per-task timeout #79715

Merged
merged 4 commits into from Jan 25, 2023
Merged

Fix: documentation for per-task timeout #79715

merged 4 commits into from Jan 25, 2023

Conversation

991jo
Copy link
Contributor

@991jo 991jo commented Jan 11, 2023

SUMMARY

Corrected the documentation regarding per-task timeouts. The timeout variable is not available in all modules in the way it is shown. See for example

- hosts: all                     
  tasks:                         
    - name: show version         
      cisco.nxos.nxos_command:   
        commands: "show version" 
        retries: 1               
        timeout: 60              

which leads to

fatal: [rcn-lab-s-2.lab.tmn.scc.kit.edu]: FAILED! => {"changed": false, "msg": "Unsupported parameters for (cisco.nxos.nxos_command) module: timeout. Supported parameters include: interval, wait_for (waitfor), retries, commands, match."}
ISSUE TYPE
  • Docs Pull Request

+label: docsite_pr

##### SUMMARY
<!--- Your description here -->

Corrected the documentation regarding per-task timeouts. The timeout variable is not available in all modules in the way it is shown.
See for example
```
- hosts: all                     
  tasks:                         
    - name: show version         
      cisco.nxos.nxos_command:   
        commands: "show version" 
        retries: 1               
        timeout: 60              
```
which leads to
```
fatal: [rcn-lab-s-2.lab.tmn.scc.kit.edu]: FAILED! => {"changed": false, "msg": "Unsupported parameters for (cisco.nxos.nxos_command) module: timeout. Supported parameters include: interval, wait_for (waitfor), retries, commands, match."}
```


##### ISSUE TYPE
- Docs Pull Request

+label: docsite_pr
@ansibot
Copy link
Contributor

ansibot commented Jan 11, 2023

Thanks for your Ansible docs contribution! We talk about Ansible documentation on matrix at #docs:ansible.im and on libera IRC at #ansible-docs if you ever want to join us and chat about the docs! We meet there on Tuesdays (see the Ansible calendar) and welcome additions to our weekly agenda items - scroll down to find the upcoming agenda and add a comment to put something new on that agenda.

click here for bot help

@ansibot ansibot added affects_2.15 docs This issue/PR relates to or includes documentation. docs_only All changes are to files within the docs/docsite/ directory docsite This issue/PR relates to the documentation website. needs_triage Needs a first human triage before being processed. networking Network category new_contributor This PR is the first contribution by a new community member. small_patch labels Jan 11, 2023
@bcoca
Copy link
Member

bcoca commented Jan 11, 2023

You are confusing the per task timeout keyword with specific module timeout options. The first applies to all modules (it is a task level keyword) and is implemented in the core engine, while the latter is implemented by the module itself.

The documentation you are updating ONLY refers to the second one.

@991jo
Copy link
Contributor Author

991jo commented Jan 11, 2023

By task level option you refer to this list: https://docs.ansible.com/ansible/latest/reference_appendices/playbooks_keywords.html#task

But then there is an error in the indentation. Currently it is:

      - name: save running-config
        cisco.ios.ios_command:
          commands: copy running-config startup-config
          provider: "{{ cli }}"
          timeout: 30

But it should be:

      - name: save running-config
        cisco.ios.ios_command:
          commands: copy running-config startup-config
          provider: "{{ cli }}"
        timeout: 30

@bcoca
Copy link
Member

bcoca commented Jan 11, 2023

yes, that is the task level one, some modules implement their own so it can be confusing and the indentation makes the difference. Also ansible_command_timeout affects ONLY the module ones that implement that (really it affects the networking additional connection plugins cli/cli_conf/httpapi/etc) ... sadly timeout is a very overloaded term.

@991jo
Copy link
Contributor Author

991jo commented Jan 11, 2023

Okay, so to summarize, there are 3 ways to do it:

  1. the task level timeout
  2. some modules might have their own timeout
  3. some module may use the ansible_command_timeout variable

I think that the documentation does not make these 3 ways and the varying support by different modules very clear.

@bcoca
Copy link
Member

bcoca commented Jan 12, 2023

  • so the 'task timeout' is universal and works at the task/host execution level
  • the per module 'timeout' is handled by the module itself
  • ansible_command_timeout is handled by the networking connection plugins, works very similar to the module version of 'timeout' but is not exactly the same

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 20, 2023
@oraNod oraNod requested a review from bcoca January 25, 2023 16:30
@ansibot ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed small_patch stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jan 25, 2023
@ansibot ansibot removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 25, 2023
@oraNod oraNod merged commit 48e6bf8 into ansible:devel Jan 25, 2023
@oraNod
Copy link
Contributor

oraNod commented Jan 25, 2023

Thanks @991jo

@samccann samccann removed the needs_triage Needs a first human triage before being processed. label Jan 26, 2023
samccann pushed a commit to samccann/ansible that referenced this pull request Jan 26, 2023
nitzmahone pushed a commit that referenced this pull request Jan 31, 2023
* fix filename for sidecar docs (#79779)

(cherry picked from commit f2707d1)

* correct examples to use removed_from_collection not collection_name (#79803)

(cherry picked from commit 7ab3de7)

* Fix: documentation for per-task timeout (#79715)

(cherry picked from commit 48e6bf8)

* [Docs] maintainers_guidelines: add WG and real-time chat request info (#79750)

(cherry picked from commit 6cb6d65)

* doc fix for platform content #79794 (#79801)

(cherry picked from commit d7a4152)

* Expand docs for the import sanity test. (#79768)

* Expand docs for the import sanity test.

* Remove note about Python 2.7 compat.

It should not be needed since there is a sanity test to enforce use of `__metaclass__ = type`.

* Improve introductory paragraph.

* Fix link typo.

(cherry picked from commit 2164d56)

* docs: Extend password entry of ansible.builtin.user (#79694)

* docs: Extend password entry of ansible.builtin.user

Clarify that `password` sets the password hash.
Not the actual password.
Fixes part of  #79684

(cherry picked from commit 6cd1a14)

* Update dev_guide.rst (#79625)

(cherry picked from commit 65eb5c0)

* Improve documentation on requirements.yml (#76140)

Makes it clear that user can use range identifiers with collection
versions inside requirements.yml files.

(cherry picked from commit 44dcfde)

---------

Co-authored-by: Evgeni Golov <evgeni@golov.de>
Co-authored-by: Jo <jo@swagspace.org>
Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>
Co-authored-by: prasadpatil49 <51715670+prasadpatil49@users.noreply.github.com>
Co-authored-by: Matt Clay <matt@mystile.com>
Co-authored-by: Hofer-Julian <30049909+Hofer-Julian@users.noreply.github.com>
Co-authored-by: Jens Timmerman <github@caret.be>
Co-authored-by: Sorin Sbarnea <ssbarnea@redhat.com>
@ansible ansible locked and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.15 docs_only All changes are to files within the docs/docsite/ directory docs This issue/PR relates to or includes documentation. docsite This issue/PR relates to the documentation website. networking Network category new_contributor This PR is the first contribution by a new community member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants