Removing Popen object return from utils.run_command_and_log
Argument 'retcode_only' of the function and associated logic was removed as it was not beneficial to the purpose, or use of the function. Before this patch, only call of the function setting 'retcode_only' to 'True' was made in tests of the tripleoclient. All other calls worked with assumption that Bool, not object will be returned, and didn't supply the argument explicitly. Instead relying on the default value. In addition, it is not clear how would the Popen[0] object be used, if it was returned. The function has limited impact across Openstack[1], only calls to it are made within tripleoclient, although there is a homonymous function withing validations_libs[2] intended for a same task. Docstring and tests were adjusted accordingly. Minor docstring typo was fixed. [0]https://docs.python.org/3/library/subprocess.html#popen-objects [1]https://codesearch.openstack.org/?q=run_command_and_log&i=nope&literal=nope&files=&excludeFiles=&repos= [2]https://opendev.org/openstack/validations-libs/src/branch/master/validations_libs/utils.py#L607 Signed-off-by: Jiri Podivin <jpodivin@redhat.com> Change-Id: Ie735ed470966a976940aefc8f99b106699080a83
This commit is contained in:
@@ -559,17 +559,6 @@ class TestRunCommandAndLog(TestCase):
|
||||
self.mock_logger.warning.assert_has_calls(self.log_calls,
|
||||
any_order=False)
|
||||
|
||||
def test_success_no_retcode(self):
|
||||
run = utils.run_command_and_log(self.mock_logger, self.cmd,
|
||||
retcode_only=False)
|
||||
self.mock_popen.assert_called_once_with(self.cmd,
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.STDOUT,
|
||||
shell=False,
|
||||
cwd=None, env=None)
|
||||
self.assertEqual(run, self.mock_process)
|
||||
self.mock_logger.warning.assert_not_called()
|
||||
|
||||
|
||||
class TestWaitForStackUtil(TestCase):
|
||||
def setUp(self):
|
||||
|
||||
@@ -2141,7 +2141,7 @@ def bulk_symlink(log, src, dst, tmpd='/tmp'):
|
||||
os.rename(tmpf, os.path.join(dst, obj))
|
||||
|
||||
|
||||
def run_command_and_log(log, cmd, cwd=None, env=None, retcode_only=True):
|
||||
def run_command_and_log(log, cmd, cwd=None, env=None):
|
||||
"""Run command and log output
|
||||
|
||||
:param log: logger instance for logging
|
||||
@@ -2150,33 +2150,28 @@ def run_command_and_log(log, cmd, cwd=None, env=None, retcode_only=True):
|
||||
:param cmd: command in list form
|
||||
:type cmd: List
|
||||
|
||||
:param cwd: current worknig directory for execution
|
||||
:param cwd: current working directory for execution
|
||||
:type cmd: String
|
||||
|
||||
:param env: modified environment for command run
|
||||
:type env: List
|
||||
|
||||
:param retcode_only: Returns only retcode instead or proc objec
|
||||
:type retcdode_only: Boolean
|
||||
"""
|
||||
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
|
||||
stderr=subprocess.STDOUT, shell=False,
|
||||
cwd=cwd, env=env)
|
||||
if retcode_only:
|
||||
while True:
|
||||
try:
|
||||
line = proc.stdout.readline()
|
||||
except StopIteration:
|
||||
break
|
||||
if line != b'':
|
||||
if isinstance(line, bytes):
|
||||
line = line.decode('utf-8')
|
||||
log.warning(line.rstrip())
|
||||
else:
|
||||
break
|
||||
proc.stdout.close()
|
||||
return proc.wait()
|
||||
return proc
|
||||
while True:
|
||||
try:
|
||||
line = proc.stdout.readline()
|
||||
except StopIteration:
|
||||
break
|
||||
if line != b'':
|
||||
if isinstance(line, bytes):
|
||||
line = line.decode('utf-8')
|
||||
log.warning(line.rstrip())
|
||||
else:
|
||||
break
|
||||
proc.stdout.close()
|
||||
return proc.wait()
|
||||
|
||||
|
||||
def build_prepare_env(environment_files, environment_directories):
|
||||
|
||||
Reference in New Issue
Block a user