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

[WIP] Issue 24215: Enable verbosity setting on a per-task basis. #41621

Open
wants to merge 5 commits into
base: devel
Choose a base branch
from

Conversation

waynr
Copy link

@waynr waynr commented Jun 16, 2018

Fixes #24215

This PR adds a new attribute/directive to the base Task class that enables users to specify verbosity level on a per-task basis, as described in #24215

It was also suggested in IRC that this behavior be added to the Block class, but I am leaving that alone for now since it's not yet clear to me how task instances can inherit from enclosing block instances.

  • Feature Pull Request

lib/ansible/playbook/task.py
lib/ansible/plugins/callback/default.py

ansible 2.7.0.dev0 (issue-24215 25ef461a44) last updated 2018/06/16 11:10:53 (GMT -500)
  config file = /home/wayne/projects/personal-networks/ansible/ansible.cfg
  configured module search path = ['/home/wayne/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/wayne/projects/ansible/ansible/lib/ansible
  executable location = /home/wayne/.virtualenvs/ansible--XOPnghv/bin/ansible
  python version = 3.6.2 (default, Sep  1 2017, 07:48:59) [GCC 6.3.0 20170516]

The current implementation in the PR branch doesn't quite work as expected, so I am submitting this WIP PR to solicit advice from those more experienced with the codebase.

Left TODO:

  • Implement test cases for the new behavior.
  • Get the new behavior working as expected.

Behavior before this change:

TASK [common : users/login | wayne | create necessary directories] *********************************************************************
ok: [agora]

TASK [common : users/login | push Makefile to userdir] *********************************************************************************
ok: [agora]

TASK [common : users/login | execute Makefile] *****************************************************************************************
changed: [agora]

Behavior after this change:

TASK [common : users/login | wayne | create necessary directories] *********************************************************************
ok: [agora]                                                                                                                             
              
TASK [common : users/login | push Makefile to userdir] *********************************************************************************
task path: /home/wayne/projects/personal-networks/ansible/roles/common/tasks/users/login/install_source.yml:9
ok: [agora] => {"changed": false, "checksum": "ced3ec0b7d5b73f3a25708d1b139a8770ab00bc8", "dest": "/home/wayne/usr/src/Makefile", "gid": 11571, "group": "wayne", "mode": "0700", "owner": "wayne", "path": "/home/wayne/usr/src/Makefile", "size": 493, "state": "file", "uid": 11571}

TASK [common : users/login | execute Makefile] *****************************************************************************************
task path: /home/wayne/projects/personal-networks/ansible/roles/common/tasks/users/login/install_source.yml:18
changed: [agora] => {"changed": true, "cmd": ["make", "all", "PREFIX=/home/wayne/usr"], "delta": "0:00:00.005516", "end": "2018-06-16 11:29:33.873340", "rc": 0, "start": "2018-06-16 11:29:33.867824", "stderr": "", "stderr_lines": [], "stdout": "make: Nothing to be done for 'all'.", "stdout_lines": ["make: Nothing to be done for 'all'."]}

Note that the first two tasks are simply ok and the third one is changed in both the before and the after examples shown above. The first task has no task-specific verbosity set; the second task has verbosity: 3, and the third has verbosity: 5.

Also, please forgive my lazyness in not putting together a full contrived example and instead using a snippet from the output of an existing playbook.

Also also, I'm not quite sure why the output shown here isn't even more verbose. The additional output resulting from this implementation doesn't match what I would expect from output when passing -vvv on the command line where the additional output shown here would be pretty print formatted and task stdout/stderr would be shown as if the tasks' command had been run locally.

@waynr waynr changed the title Issue 24215: Enable verbosity setting on a per-task basis. [WIP] Issue 24215: Enable verbosity setting on a per-task basis. Jun 16, 2018
@@ -152,7 +152,7 @@ def _print_task_banner(self, task):
args = u' %s' % args

self._display.banner(u"TASK [%s%s]" % (task.get_name().strip(), args))
if self._display.verbosity >= 2:
if self._display.verbosity >= 2 or task.verbosity >= 2:
Copy link
Author

@waynr waynr Jun 16, 2018

Choose a reason for hiding this comment

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

Currently it seems like this is the only place where the task-specific increased verbosity is working as intended.

Copy link
Author

@waynr waynr Jun 16, 2018

Choose a reason for hiding this comment

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

Nevermind, I figured it out.

@ansibot ansibot added WIP affects_2.7 feature needs_triage new_contributor support:core labels Jun 16, 2018
@ansibot
Copy link
Contributor

@ansibot ansibot commented Jun 16, 2018

The test ansible-test sanity --test pep8 [explain] failed with 5 errors:

lib/ansible/plugins/callback/default.py:97:161: E501 line too long (176 > 160 characters)
lib/ansible/plugins/callback/default.py:113:161: E501 line too long (180 > 160 characters)
lib/ansible/plugins/callback/default.py:210:161: E501 line too long (172 > 160 characters)
lib/ansible/plugins/callback/default.py:233:161: E501 line too long (176 > 160 characters)
lib/ansible/plugins/callback/default.py:302:161: E501 line too long (172 > 160 characters)

click here for bot help

@ansibot ansibot added the ci_verified label Jun 16, 2018
@ansibot ansibot removed the ci_verified label Jun 17, 2018
@ansibot
Copy link
Contributor

@ansibot ansibot commented Jun 17, 2018

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

lib/ansible/plugins/callback/default.py:208:11: undefined-variable Undefined variable 'task'
lib/ansible/plugins/callback/default.py:236:11: undefined-variable Undefined variable 'task'
lib/ansible/plugins/callback/default.py:253:11: undefined-variable Undefined variable 'task'

click here for bot help

@bcoca bcoca removed the needs_triage label Jun 18, 2018
@ansibot ansibot added needs_rebase stale_ci labels Jun 26, 2018
@ansibot ansibot added the support:community label Sep 14, 2018
@gundalow
Copy link
Contributor

@gundalow gundalow commented Jan 11, 2019

We already allow no_log per task, so this is something that makes sense

display is a singleton now, and if you want to do threading, they need to restore verbosity

This PR may not apply to blocks when it should and verbosity should be inherited up

@michaelsmoody
Copy link

@michaelsmoody michaelsmoody commented Mar 13, 2020

As #68084 was closed, and this one seems the most relevant, it might be useful to bump the verbosity of a task up from normal, so that I can have my own debug verbosity setting not get overran by other modules. As an example, I want to add some additional data with a verbosity of 1 to some debug statements, but when doing so, using a module such as win_robocopy, or synchronize, or some other module which might make lots and lots of operations, I would like to modify the minimum verbosity of synchronize or win_robocopy output to 2, instead of 1, so that I can implement my own debug without getting literally megabytes of console output.

I'm certainly open to other suggestions that might make this possible if this is not appropriate.

@ansibot ansibot added collection collection:community.general labels Apr 29, 2020
@tpo
Copy link
Contributor

@tpo tpo commented Aug 8, 2020

This is a feature that I would have used today.

However this pull request is two years old and there isn't a comment from anybody with commit rights on:

  • whether they agree with the basic idea of this pull request?
  • if they do agree, if there's something missing or that should be added (block level verbosity could be added, is it required though before merging this?)?
  • anything else?

Could somebody that has commit rights please let the people here know what the plan is wrt to this pull request?

@waynr
Copy link
Author

@waynr waynr commented Aug 8, 2020

I'd be happy to rebase this onto the latest devel branch if one of the maintainers does happen to be open to it. I had totally forgotten about it until I got an email notification about the latest comment.

@ansibot ansibot added pre_azp and removed stale_ci labels Dec 7, 2020
@ansibot ansibot added bug and removed collection:community.general labels Jan 24, 2021
@ansibot ansibot removed the support:community label Mar 5, 2021
@frittentheke
Copy link
Contributor

@frittentheke frittentheke commented May 14, 2021

I'd be happy to rebase this onto the latest devel branch if one of the maintainers does happen to be open to it. I had totally forgotten about it until I got an email notification about the latest comment.

@waynr this PR is just so obviously helpful - thank you!
Do you mind rebasing it once more just for the sake of it to have a clean CI and the potential to have it merged?

@s-hertel s-hertel mentioned this pull request Jun 3, 2021
@bcoca
Copy link
Member

@bcoca bcoca commented Apr 8, 2022

A few issues with this:

  • it only addresses a small part of the possible information involved in a task (nothing on action/connection/shell/become plugins or modules, which produce most of the output)
  • it is only at 'task' level, block/role/play setting should be the default for most new features
  • ignores 'templatability' and assumes users will only want a hardcoded value for a specific task (why not just the problem host?)
    verbosity: '{{ inventory_hostname in ('problem1', 'problem2')|ternary(3, 0)|int}}'

The main reason we have not implemented this is due to the lack of 'single source of verbosity truth' .. which is what this accomplishes #77498.

Now we can deal with verbosity to ALMOST cover the 'whole task' when executing .. still leaves the callback as a problem as it executes in a different process/context (but moving those to rely on task value instead of display as this PR does might solve that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.7 bug collection feature has_issue needs_rebase new_contributor pre_azp support:core WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants