From acb55ef279cd5ae0f489f6b5298258d5ff1a437e Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Tue, 13 Jan 2015 17:37:59 +0000 Subject: [PATCH] Revert "Port processutils to Python 3" This reverts commit 7c7493feb53429577efca2c4b0380af03ddc149b. This change breaks the API of execute() and breaks nova's tests. Closes-Bug: #1410348 Change-Id: I33fa9b4e6099f521e0de3cb4585016fd694c4548 --- oslo_concurrency/processutils.py | 21 +------------------ .../tests/unit/test_processutils.py | 16 +++++++------- tests/test_processutils.py | 16 +++++++------- 3 files changed, 17 insertions(+), 36 deletions(-) diff --git a/oslo_concurrency/processutils.py b/oslo_concurrency/processutils.py index 78f8f14..051acec 100644 --- a/oslo_concurrency/processutils.py +++ b/oslo_concurrency/processutils.py @@ -23,7 +23,6 @@ import os import random import shlex import signal -import sys import time from oslo.utils import importutils @@ -143,13 +142,6 @@ def execute(*cmd, **kwargs): last attempt, and LOG_ALL_ERRORS requires logging on each occurence of an error. :type log_errors: integer. - :param encoding: encoding used to decode stdout and stderr, - sys.getfilesystemencoding() by default. - :type encoding: str - :param errors: error handler used to decode stdout and stderr, - default: 'surrogateescape' on Python 3, - 'strict' on Python 2. - :type errors: str :returns: (stdout, stderr) from process execution :raises: :class:`UnknownArgumentError` on receiving unknown arguments @@ -168,8 +160,6 @@ def execute(*cmd, **kwargs): shell = kwargs.pop('shell', False) loglevel = kwargs.pop('loglevel', logging.DEBUG) log_errors = kwargs.pop('log_errors', None) - encoding = kwargs.pop('encoding', sys.getfilesystemencoding()) - errors = kwargs.pop('errors', 'surrogateescape' if six.PY3 else 'strict') if isinstance(check_exit_code, bool): ignore_exit_code = not check_exit_code @@ -224,11 +214,6 @@ def execute(*cmd, **kwargs): end_time = time.time() - start_time LOG.log(loglevel, 'CMD "%s" returned: %s in %ss' % (sanitized_cmd, _returncode, end_time)) - if result is not None: - (stdout, stderr) = result - stdout = stdout.decode(encoding, errors) - stderr = stderr.decode(encoding, errors) - result = (stdout, stderr) if not ignore_exit_code and _returncode not in check_exit_code: (stdout, stderr) = result sanitized_stdout = strutils.mask_password(stdout) @@ -307,9 +292,7 @@ def trycmd(*args, **kwargs): def ssh_execute(ssh, cmd, process_input=None, - addl_env=None, check_exit_code=True, - encoding=sys.getfilesystemencoding(), - errors='surrogateescape' if six.PY3 else 'strict'): + addl_env=None, check_exit_code=True): sanitized_cmd = strutils.mask_password(cmd) LOG.debug('Running cmd (SSH): %s', sanitized_cmd) if addl_env: @@ -325,10 +308,8 @@ def ssh_execute(ssh, cmd, process_input=None, # NOTE(justinsb): This seems suspicious... # ...other SSH clients have buffering issues with this approach stdout = stdout_stream.read() - stdout = stdout.decode(encoding, errors) sanitized_stdout = strutils.mask_password(stdout) stderr = stderr_stream.read() - stderr = stderr.decode(encoding, errors) sanitized_stderr = strutils.mask_password(stderr) stdin_stream.close() diff --git a/oslo_concurrency/tests/unit/test_processutils.py b/oslo_concurrency/tests/unit/test_processutils.py index 65b4e2d..50ea611 100644 --- a/oslo_concurrency/tests/unit/test_processutils.py +++ b/oslo_concurrency/tests/unit/test_processutils.py @@ -295,7 +295,7 @@ grep foo out, err = processutils.execute('/usr/bin/env', env_variables=env_vars) - self.assertIn('SUPER_UNIQUE_VAR=The answer is 42', out) + self.assertIn(b'SUPER_UNIQUE_VAR=The answer is 42', out) def test_exception_and_masking(self): tmpfilename = self.create_tempfiles( @@ -426,7 +426,7 @@ class FakeSshChannel(object): return self.rc -class FakeSshStream(six.BytesIO): +class FakeSshStream(six.StringIO): def setup_channel(self, rc): self.channel = FakeSshChannel(rc) @@ -436,11 +436,11 @@ class FakeSshConnection(object): self.rc = rc def exec_command(self, cmd): - stdout = FakeSshStream(b'stdout') + stdout = FakeSshStream('stdout') stdout.setup_channel(self.rc) - return (six.BytesIO(), + return (six.StringIO(), stdout, - six.BytesIO(b'stderr')) + six.StringIO('stderr')) class SshExecuteTestCase(test_base.BaseTestCase): @@ -465,13 +465,13 @@ class SshExecuteTestCase(test_base.BaseTestCase): def _test_compromising_ssh(self, rc, check): fixture = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG)) - fake_stdin = six.BytesIO() + fake_stdin = six.StringIO() fake_stdout = mock.Mock() fake_stdout.channel.recv_exit_status.return_value = rc - fake_stdout.read.return_value = b'password="secret"' + fake_stdout.read.return_value = 'password="secret"' - fake_stderr = six.BytesIO(b'password="foobar"') + fake_stderr = six.StringIO('password="foobar"') command = 'ls --password="bar"' diff --git a/tests/test_processutils.py b/tests/test_processutils.py index ca15385..179a5e3 100644 --- a/tests/test_processutils.py +++ b/tests/test_processutils.py @@ -296,7 +296,7 @@ grep foo out, err = processutils.execute('/usr/bin/env', env_variables=env_vars) - self.assertIn('SUPER_UNIQUE_VAR=The answer is 42', out) + self.assertIn(b'SUPER_UNIQUE_VAR=The answer is 42', out) def test_exception_and_masking(self): tmpfilename = self.create_tempfiles( @@ -427,7 +427,7 @@ class FakeSshChannel(object): return self.rc -class FakeSshStream(six.BytesIO): +class FakeSshStream(six.StringIO): def setup_channel(self, rc): self.channel = FakeSshChannel(rc) @@ -437,11 +437,11 @@ class FakeSshConnection(object): self.rc = rc def exec_command(self, cmd): - stdout = FakeSshStream(b'stdout') + stdout = FakeSshStream('stdout') stdout.setup_channel(self.rc) - return (six.BytesIO(), + return (six.StringIO(), stdout, - six.BytesIO(b'stderr')) + six.StringIO('stderr')) class SshExecuteTestCase(test_base.BaseTestCase): @@ -466,13 +466,13 @@ class SshExecuteTestCase(test_base.BaseTestCase): def _test_compromising_ssh(self, rc, check): fixture = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG)) - fake_stdin = six.BytesIO() + fake_stdin = six.StringIO() fake_stdout = mock.Mock() fake_stdout.channel.recv_exit_status.return_value = rc - fake_stdout.read.return_value = b'password="secret"' + fake_stdout.read.return_value = 'password="secret"' - fake_stderr = six.BytesIO(b'password="foobar"') + fake_stderr = six.StringIO('password="foobar"') command = 'ls --password="bar"'