Respect user-specified stdout callback #387

AlanCoding · 2019-11-11T15:10:36Z

No description provided.

strider · 2019-12-09T11:59:30Z

Tested this PR because I need it and it works as expected! +1

cloudnull

👍 this seems to work as expected

strider · 2020-01-02T14:41:24Z

Hi @AlanCoding,
Any chance to get this PR merged ASAP?

AlanCoding · 2020-01-02T14:55:04Z

While this is probably functional, I don't think a near-term merge is viable. Here is what I would like to do and have been pitching to people when I get a chance:

The content in https://github.com/ansible/ansible-runner/tree/master/ansible_runner/display_callback is shared code in multiple display callbacks (pretty much just job vs. ad hoc commands). The imports work the way they do today because this was all written before Ansible supported user-defined module_utils paths.

This PR does some ugly stuff juggling the DOCUMENTATION string because of the way that imports are done. If we restructure the code to the more proper format without PATH hacks, then we won't need that, and this feature will look almost trivial.

However, adding to the module_utils paths will almost certain break some use cases where the user is already using some custom module_utils paths. So we need to pull in some other code (originally done in AWX) to respect user-defined configs when ansible-runner itself relies on some config vars for basic operation.

So basically, I want these two other items in to pay down some tech debt before truly supporting this. I do very strongly want to do those. It is viable, but less trivial than the hacky stuff here.

cloudnull · 2020-01-03T15:42:30Z

Hi @AlanCoding, maybe we could sync up on IRC (strider, cloudnull), or on the mailing list, to talk about how we can help. We're now using ansible-runner within the OpenStack TripleO project and having the ability to set user defined callbacks is important to us. If we can help usher this feature along by working on the other two items we'd be happy to do so.

ansible-zuul · 2020-02-10T16:30:05Z

Build succeeded.

github-workflows : SUCCESS in 50s
ansible-tox-linters : SUCCESS in 5m 50s
ansible-tox-py36 : SUCCESS in 9m 16s
ansible-tox-py37 : SUCCESS in 9m 28s
ansible-build-python-tarball : SUCCESS in 3m 26s
ansible-tox-py27 : SUCCESS in 8m 13s

AlanCoding · 2020-02-10T16:37:20Z

I'm ready to take this out of draft and call it ready for review. Before I do that, I will squash commits, but I made this branch to preserve this prior history:

https://github.com/ansible/ansible-runner/compare/devel...AlanCoding:stdout_callback_backup?expand=1

So after this comment, cleanup of this PR will start.

Create new integration test data folder this contains folders of runnable content contains example with selective callback contains example not printing skipped hosts Fix bugs combining display callbacks due to call pattern if parent class took var as kwarg, then passing as arg will stop working when another parent is added in the inheritance chain Restructure DOCUMENTATION management better align with user-specified display callback Delete parameterization to test aggregate callback gave false positives and generally did not work replaced with new test and data example

AlanCoding · 2020-02-10T16:46:10Z

ansible_runner/display_callback/module.py

+    def get_documentation(cls):
+        if hasattr(cls, 'PROXY_DOCUMENTATION'):
+            return cls.PROXY_DOCUMENTATION
+        return BASE_DOCUMENTATION % cls.CALLBACK_NAME


My ears are open if someone wants to suggest a more beautiful way of creating the DOCUMENTATION string in this specific logic.

The "perfect" solution would be for awx_display and minimal to both extend from the same doc_fragment (which would still need to be overwritten by the user callback, if present). But that won't be available until we are fully in the world of collections. So to me, the general thing being doing here seems fully justifiable.

AlanCoding · 2020-02-10T16:46:31Z

ansible_runner/display_callback/module.py

@@ -374,7 +404,7 @@ def v2_runner_on_failed(self, result, ignore_errors=False):
            event_loop=self._get_event_loop(result._task),
        )
        with self.capture_event_data('runner_on_failed', **event_data):
-            super(BaseCallbackModule, self).v2_runner_on_failed(result, ignore_errors)
+            super(BaseCallbackModule, self).v2_runner_on_failed(result, ignore_errors=ignore_errors)


in Ansible core, these are kwargs, not args. Thus the change.

AlanCoding · 2020-02-10T16:47:51Z

ansible_runner/runner_config.py

@@ -195,6 +195,9 @@ def prepare(self):
        if python_path and not python_path.endswith(':'):
            python_path += ':'
        self.env['ANSIBLE_CALLBACK_PLUGINS'] = ':'.join(filter(None,(self.env.get('ANSIBLE_CALLBACK_PLUGINS'), callback_dir)))
+        # If stdout callback specified, pass info for this stdout callback plugin to subclass it
+        if 'ANSIBLE_STDOUT_CALLBACK' in self.env and self.env['ANSIBLE_STDOUT_CALLBACK'] not in ('awx_display', 'minimal'):
+            self.env['RUNNER_STDOUT_CALLBACK_PROXY'] = self.env['ANSIBLE_STDOUT_CALLBACK']


Needed this name RUNNER_STDOUT_CALLBACK_PROXY to be very specific. It could have a 20 digit uuid in its name for all I care, it's only passing it from parent context to child context and is a single static string.

AlanCoding · 2020-02-10T16:49:41Z

test/data/callback_options/project/task_status.yml

+    - name: Reach out to the unreachable host
+      ping:
+      vars:
+        ansible_host: 'invalid.invalid'


Somewhere else I'm going with this - I want to test @wenottingham's #405 with it, because I'm worried about our ability (my ability) to correctly identify all task-ending callback types.

AlanCoding · 2020-02-10T16:50:37Z

test/data/display_callback/project/task_status.yml

+    - name: Reach out to the unreachable host
+      ping:
+      vars:
+        ansible_host: 'invalid.invalid'


yeah, I don't like the duplication either, but runner right now does not like to combine programatic and file inputs, so that's why it ended up like this.

ansible-zuul · 2020-02-10T16:52:58Z

Build succeeded.

github-workflows : SUCCESS in 49s
ansible-tox-linters : SUCCESS in 5m 07s
ansible-tox-py36 : SUCCESS in 9m 45s
ansible-tox-py37 : SUCCESS in 9m 27s
ansible-build-python-tarball : SUCCESS in 3m 22s
ansible-tox-py27 : SUCCESS in 8m 22s

ryanpetrello

This seems pretty reasonable to me.

matburt · 2020-02-13T16:37:19Z

test/data/README.md

@@ -0,0 +1,15 @@
+### Ansible Runner Test Data Directories
+
+Subfolders in this directory should contain test cases in the form of


ansible-zuul · 2020-02-14T02:23:19Z

Build succeeded (gate pipeline).

github-workflows : SUCCESS in 54s
ansible-tox-linters : SUCCESS in 5m 06s
ansible-tox-py36 : SUCCESS in 9m 13s
ansible-tox-py37 : SUCCESS in 10m 08s
ansible-build-python-tarball : SUCCESS in 4m 05s
ansible-tox-py27 : SUCCESS in 11m 03s

AlanCoding · 2020-02-17T15:59:20Z

This was mistakenly merged into the master branch, which changed to the devel branch at some point. That's not good.

AlanCoding force-pushed the stdout_callback branch 2 times, most recently from 3da093b to 15a0814 Compare November 12, 2019 14:39

AlanCoding mentioned this pull request Nov 22, 2019

UI Improvement for suppressing ok from the result of job output ansible/awx#4092

Open

cloudnull approved these changes Dec 9, 2019

View reviewed changes

AlanCoding mentioned this pull request Jan 29, 2020

suppressing long job output like selective stdout callback ansible/awx#5789

Open

wenottingham mentioned this pull request Feb 5, 2020

Respect stdout callback being set #400

Closed

AlanCoding force-pushed the stdout_callback branch from 15a0814 to cf16133 Compare February 8, 2020 03:58

AlanCoding force-pushed the stdout_callback branch from 124cf23 to 1cc53bf Compare February 10, 2020 16:41

ansible deleted a comment from ansible-zuul bot Feb 10, 2020

AlanCoding commented Feb 10, 2020

View reviewed changes

AlanCoding marked this pull request as ready for review February 10, 2020 16:51

AlanCoding requested a review from matburt February 10, 2020 17:02

ryanpetrello approved these changes Feb 11, 2020

View reviewed changes

matburt approved these changes Feb 13, 2020

View reviewed changes

AlanCoding added the gate label Feb 14, 2020

ansible-zuul bot merged commit 9e7e75d into ansible:master Feb 14, 2020

This was referenced Feb 17, 2020

Fix duplicated output in stats task #411

Closed

Allow customizing display callback, refactor test data #412

Closed

AlanCoding mentioned this pull request Dec 16, 2021

Support custom stdout callbacks #763

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Respect user-specified stdout callback #387

Respect user-specified stdout callback #387

AlanCoding commented Nov 11, 2019

strider commented Dec 9, 2019

cloudnull left a comment

strider commented Jan 2, 2020

AlanCoding commented Jan 2, 2020

cloudnull commented Jan 3, 2020

ansible-zuul bot commented Feb 10, 2020

AlanCoding commented Feb 10, 2020

AlanCoding Feb 10, 2020

AlanCoding Feb 10, 2020

AlanCoding Feb 10, 2020

AlanCoding Feb 10, 2020

AlanCoding Feb 10, 2020

ansible-zuul bot commented Feb 10, 2020

ryanpetrello left a comment

matburt Feb 13, 2020

ansible-zuul bot commented Feb 14, 2020

AlanCoding commented Feb 17, 2020

		@@ -0,0 +1,15 @@
		### Ansible Runner Test Data Directories

		Subfolders in this directory should contain test cases in the form of

Respect user-specified stdout callback #387

Respect user-specified stdout callback #387

Conversation

AlanCoding commented Nov 11, 2019

strider commented Dec 9, 2019

cloudnull left a comment

Choose a reason for hiding this comment

strider commented Jan 2, 2020

AlanCoding commented Jan 2, 2020

cloudnull commented Jan 3, 2020

ansible-zuul bot commented Feb 10, 2020

AlanCoding commented Feb 10, 2020

AlanCoding Feb 10, 2020

Choose a reason for hiding this comment

AlanCoding Feb 10, 2020

Choose a reason for hiding this comment

AlanCoding Feb 10, 2020

Choose a reason for hiding this comment

AlanCoding Feb 10, 2020

Choose a reason for hiding this comment

AlanCoding Feb 10, 2020

Choose a reason for hiding this comment

ansible-zuul bot commented Feb 10, 2020

ryanpetrello left a comment

Choose a reason for hiding this comment

matburt Feb 13, 2020

Choose a reason for hiding this comment

ansible-zuul bot commented Feb 14, 2020

AlanCoding commented Feb 17, 2020