diff options
author | Jenkins <jenkins@review.openstack.org> | 2015-07-06 21:22:17 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2015-07-06 21:22:17 +0000 |
commit | 7243f588103c3728172dde23ebebae218fc51046 (patch) | |
tree | ebdcc79940b71f84b85abdd9c0dafa8182442aa3 | |
parent | 43722464b5709af7a37ce050f2446cd992a61041 (diff) | |
parent | ab78480659834ed07c84f8569df5d2c27965af00 (diff) | |
download | oslo-concurrency-7243f588103c3728172dde23ebebae218fc51046.tar.gz |
Merge "processutils: ensure on_completion callback is always called"
-rw-r--r-- | oslo_concurrency/processutils.py | 19 | ||||
-rw-r--r-- | oslo_concurrency/tests/unit/test_processutils.py | 19 |
2 files changed, 29 insertions, 9 deletions
diff --git a/oslo_concurrency/processutils.py b/oslo_concurrency/processutils.py index 0fd7915..01c02db 100644 --- a/oslo_concurrency/processutils.py +++ b/oslo_concurrency/processutils.py @@ -239,15 +239,16 @@ def execute(*cmd, **kwargs): if on_execute: on_execute(obj) - result = obj.communicate(process_input) - - obj.stdin.close() # pylint: disable=E1101 - _returncode = obj.returncode # pylint: disable=E1101 - LOG.log(loglevel, 'CMD "%s" returned: %s in %0.3fs', - sanitized_cmd, _returncode, watch.elapsed()) - - if on_completion: - on_completion(obj) + try: + result = obj.communicate(process_input) + + obj.stdin.close() # pylint: disable=E1101 + _returncode = obj.returncode # pylint: disable=E1101 + LOG.log(loglevel, 'CMD "%s" returned: %s in %0.3fs', + sanitized_cmd, _returncode, watch.elapsed()) + finally: + if on_completion: + on_completion(obj) if not ignore_exit_code and _returncode not in check_exit_code: (stdout, stderr) = result diff --git a/oslo_concurrency/tests/unit/test_processutils.py b/oslo_concurrency/tests/unit/test_processutils.py index 361d909..771d966 100644 --- a/oslo_concurrency/tests/unit/test_processutils.py +++ b/oslo_concurrency/tests/unit/test_processutils.py @@ -20,6 +20,7 @@ import logging import multiprocessing import os import stat +import subprocess import sys import tempfile @@ -78,6 +79,24 @@ class UtilsTest(test_base.BaseTestCase): self.assertEqual(1, on_execute_callback.call_count) self.assertEqual(1, on_completion_callback.call_count) + @mock.patch.object(subprocess.Popen, "communicate") + def test_execute_with_callback_and_errors(self, mock_comm): + on_execute_callback = mock.Mock() + on_completion_callback = mock.Mock() + + def fake_communicate(*args): + raise IOError("Broken pipe") + + mock_comm.side_effect = fake_communicate + + self.assertRaises(IOError, + processutils.execute, + "/bin/true", + on_execute=on_execute_callback, + on_completion=on_completion_callback) + self.assertEqual(1, on_execute_callback.call_count) + self.assertEqual(1, on_completion_callback.call_count) + class ProcessExecutionErrorTest(test_base.BaseTestCase): |