From a8d835269b0d3b4c58a2c1bf78b7e369127ff73d Mon Sep 17 00:00:00 2001 From: Yu Ming Zhu Date: Jan 31 2019 19:31:21 +0000 Subject: [PATCH 1/4] using ConfigParser.read_file for PY3 --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index d4462bd..3896695 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -5821,10 +5821,7 @@ def handle_image_build(options, session, args): if not os.path.exists(task_options.config): parser.error(_("%s not found!" % task_options.config)) section = 'image-build' - config = six.moves.configparser.ConfigParser() - conf_fd = open(task_options.config) - config.readfp(conf_fd) - conf_fd.close() + config = koji.read_config_files(task_options.config) if not config.has_section(section): parser.error(_("single section called [%s] is required" % section)) # pluck out the positional arguments first diff --git a/koji/__init__.py b/koji/__init__.py index 05c8e5f..9790f64 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1698,10 +1698,7 @@ def read_config(profile_name, user_config=None): # Load the configs in a particular order got_conf = False for configFile in configs: - f = open(configFile) - config = six.moves.configparser.ConfigParser() - config.readfp(f) - f.close() + config = read_config_files(configFile) if config.has_section(profile_name): got_conf = True for name, value in config.items(profile_name): @@ -1795,6 +1792,21 @@ def get_profile_module(profile_name, config=None): return mod +def read_config_files(config_files, parser=None): + if not isinstance(config_files, (list, tuple)): + config_files = [config_files] + if parser is None: + parser = six.moves.configparser.ConfigParser + config = parser() + for config_file in config_files: + with open(config_file, 'r') as f: + if six.PY2: + config.readfp(f) + else: + config.read_file(f) + return config + + class PathInfo(object): # ASCII numbers and upper- and lower-case letter for use in tmpdir() ASCII_CHARS = [chr(i) for i in list(range(48, 58)) + list(range(65, 91)) + list(range(97, 123))] diff --git a/koji/util.py b/koji/util.py index deee951..f37d6a3 100644 --- a/koji/util.py +++ b/koji/util.py @@ -733,13 +733,7 @@ def parse_maven_params(confs, chain=False, scratch=False): Return a map whose keys are package names and values are config parameters. """ - if not isinstance(confs, (list, tuple)): - confs = [confs] - config = six.moves.configparser.ConfigParser() - for conf in confs: - conf_fd = open(conf) - config.readfp(conf_fd) - conf_fd.close() + config = koji.read_config_files(confs) builds = {} for package in config.sections(): buildtype = 'maven' @@ -753,10 +747,12 @@ def parse_maven_params(confs, chain=False, scratch=False): raise ValueError("A wrapper-rpm must depend on exactly one package") else: raise ValueError("Unsupported build type: %s" % buildtype) - if not 'scmurl' in params: + if 'scmurl' not in params: raise ValueError("%s is missing the scmurl parameter" % package) builds[package] = params if not builds: + if not isinstance(confs, (list, tuple)): + confs = [confs] raise ValueError("No sections found in: %s" % ', '.join(confs)) return builds diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py index 7a21ace..462da96 100644 --- a/plugins/hub/protonmsg.py +++ b/plugins/hub/protonmsg.py @@ -270,10 +270,8 @@ def send_queued_msgs(cbtype, *args, **kws): log = logging.getLogger('koji.plugin.protonmsg') global CONFIG if not CONFIG: - conf = six.moves.configparser.SafeConfigParser() - with open(CONFIG_FILE) as conffile: - conf.readfp(conffile) - CONFIG = conf + CONFIG = koji.read_config_files(CONFIG_FILE, + six.moves.configparser.SafeConfigParser) urls = CONFIG.get('broker', 'urls').split() test_mode = False if CONFIG.has_option('broker', 'test_mode'): diff --git a/tests/test_lib/test_utils.py b/tests/test_lib/test_utils.py index 02e5d32..4d24f49 100644 --- a/tests/test_lib/test_utils.py +++ b/tests/test_lib/test_utils.py @@ -158,6 +158,47 @@ class MiscFunctionTestCase(unittest.TestCase): m.assert_not_called() +class ConfigFileTestCase(unittest.TestCase): + """Test config file reading functions""" + + @mock_open() + @mock.patch("six.moves.configparser.ConfigParser", spec=True) + @mock.patch("six.moves.configparser.SafeConfigParser", spec=True) + def test_read_config_files(self, scp_clz, cp_clz, open_mock): + files = 'test1.conf' + conf = koji.read_config_files(files) + self.assertTrue(isinstance(conf, + six.moves.configparser.ConfigParser.__class__)) + cp_clz.assert_called_once() + open_mock.assert_called_once_with(files, 'r') + if six.PY2: + cp_clz.return_value.readfp.assert_called_once() + else: + cp_clz.return_value.read_file.assert_called_once() + + open_mock.reset_mock() + cp_clz.reset_mock() + files = ['test1.conf', 'test2.conf'] + koji.read_config_files(files) + cp_clz.assert_called_once() + open_mock.assert_has_calls([call('test1.conf', 'r'), + call('test2.conf', 'r')], + any_order=True) + if six.PY2: + self.assertEqual(cp_clz.return_value.readfp.call_count, 2) + else: + self.assertEqual(cp_clz.return_value.read_file.call_count, 2) + + open_mock.reset_mock() + cp_clz.reset_mock() + conf = koji.read_config_files(files, + six.moves.configparser.SafeConfigParser) + self.assertTrue(isinstance(conf, + six.moves.configparser.SafeConfigParser.__class__)) + cp_clz.assert_not_called() + scp_clz.assert_called_once() + + class MavenUtilTestCase(unittest.TestCase): """Test maven relative functions""" maxDiff = None @@ -494,7 +535,10 @@ class MavenUtilTestCase(unittest.TestCase): config = six.moves.configparser.ConfigParser() path = os.path.dirname(__file__) with open(path + cfile, 'r') as conf_file: - config.readfp(conf_file) + if six.PY2: + config.readfp(conf_file) + else: + config.read_file(conf_file) return config def test_formatChangelog(self): diff --git a/tests/test_plugins/test_protonmsg.py b/tests/test_plugins/test_protonmsg.py index 1414bee..5837891 100644 --- a/tests/test_plugins/test_protonmsg.py +++ b/tests/test_plugins/test_protonmsg.py @@ -270,7 +270,10 @@ connect_timeout = 10 send_timeout = 60 """) conf = SafeConfigParser() - conf.readfp(confdata) + if six.PY2: + conf.readfp(confdata) + else: + conf.read_file(confdata) self.handler = protonmsg.TimeoutHandler('amqps://broker1.example.com:5671', [], conf) @patch('protonmsg.SSLDomain') @@ -291,7 +294,10 @@ connect_timeout = 10 send_timeout = 60 """) conf = SafeConfigParser() - conf.readfp(confdata) + if six.PY2: + conf.readfp(confdata) + else: + conf.read_file(confdata) handler = protonmsg.TimeoutHandler('amqp://broker1.example.com:5672', [], conf) event = MagicMock() handler.on_start(event) From c91e6e66ce262ef3610c1ac95a55dacc25dc83bc Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Jan 31 2019 19:31:21 +0000 Subject: [PATCH 2/4] docstring for read_config_files --- diff --git a/koji/__init__.py b/koji/__init__.py index 9790f64..c99dabd 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1793,6 +1793,15 @@ def get_profile_module(profile_name, config=None): def read_config_files(config_files, parser=None): + """Use parser to read config file(s) + + :param config_files: config file(s) to read (required). + :type config_files: str or list + :param type parser: class/sub-class of `configparser.RawConfigParser`. + If it's None or omitted, using + `six.moves.configparser.ConfigParser` as default. + :return: object of parser which contains parsed content + """ if not isinstance(config_files, (list, tuple)): config_files = [config_files] if parser is None: From 0ae78c1d8da1ead9bce76688b80cdf67a66a9107 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 31 2019 19:33:57 +0000 Subject: [PATCH 3/4] drop parser option in favor of raw option --- diff --git a/koji/__init__.py b/koji/__init__.py index c99dabd..7cd2659 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -1792,19 +1792,24 @@ def get_profile_module(profile_name, config=None): return mod -def read_config_files(config_files, parser=None): +def read_config_files(config_files, raw=False): """Use parser to read config file(s) :param config_files: config file(s) to read (required). :type config_files: str or list - :param type parser: class/sub-class of `configparser.RawConfigParser`. - If it's None or omitted, using - `six.moves.configparser.ConfigParser` as default. + :param bool raw: enable 'raw' parsing (no interpolation). Default: False + :return: object of parser which contains parsed content """ if not isinstance(config_files, (list, tuple)): config_files = [config_files] - if parser is None: + if raw: + parser = six.moves.configparser.RawConfigParser + elif six.PY2: + parser = six.moves.configparser.SafeConfigParser + else: + # In python3, ConfigParser is "safe", and SafeConfigParser is a + # deprecated alias parser = six.moves.configparser.ConfigParser config = parser() for config_file in config_files: From ac9ae614c60649a97ad28cd9df38355710d5025b Mon Sep 17 00:00:00 2001 From: Yu Ming Zhu Date: Jan 31 2019 20:13:50 +0000 Subject: [PATCH 4/4] fix invokings and unittests --- diff --git a/koji/util.py b/koji/util.py index f37d6a3..f0d9cc3 100644 --- a/koji/util.py +++ b/koji/util.py @@ -36,7 +36,6 @@ import stat import struct import sys import time -import six.moves.configparser from zlib import adler32 from six.moves import range import six diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py index 462da96..4953529 100644 --- a/plugins/hub/protonmsg.py +++ b/plugins/hub/protonmsg.py @@ -9,7 +9,6 @@ from __future__ import absolute_import import koji from koji.plugin import callback, ignore_error, convert_datetime from koji.context import context -import six.moves.configparser import logging import json import random @@ -270,8 +269,7 @@ def send_queued_msgs(cbtype, *args, **kws): log = logging.getLogger('koji.plugin.protonmsg') global CONFIG if not CONFIG: - CONFIG = koji.read_config_files(CONFIG_FILE, - six.moves.configparser.SafeConfigParser) + CONFIG = koji.read_config_files(CONFIG_FILE) urls = CONFIG.get('broker', 'urls').split() test_mode = False if CONFIG.has_option('broker', 'test_mode'): diff --git a/tests/test_lib/test_utils.py b/tests/test_lib/test_utils.py index 4d24f49..ff78112 100644 --- a/tests/test_lib/test_utils.py +++ b/tests/test_lib/test_utils.py @@ -163,40 +163,47 @@ class ConfigFileTestCase(unittest.TestCase): @mock_open() @mock.patch("six.moves.configparser.ConfigParser", spec=True) + @mock.patch("six.moves.configparser.RawConfigParser", spec=True) @mock.patch("six.moves.configparser.SafeConfigParser", spec=True) - def test_read_config_files(self, scp_clz, cp_clz, open_mock): + def test_read_config_files(self, scp_clz, rcp_clz, cp_clz, open_mock): files = 'test1.conf' conf = koji.read_config_files(files) - self.assertTrue(isinstance(conf, - six.moves.configparser.ConfigParser.__class__)) - cp_clz.assert_called_once() open_mock.assert_called_once_with(files, 'r') if six.PY2: - cp_clz.return_value.readfp.assert_called_once() + self.assertTrue(isinstance(conf, + six.moves.configparser.SafeConfigParser.__class__)) + scp_clz.assert_called_once() + scp_clz.return_value.readfp.assert_called_once() else: + self.assertTrue(isinstance(conf, + six.moves.configparser.ConfigParser.__class__)) + cp_clz.assert_called_once() cp_clz.return_value.read_file.assert_called_once() open_mock.reset_mock() cp_clz.reset_mock() + scp_clz.reset_mock() files = ['test1.conf', 'test2.conf'] koji.read_config_files(files) - cp_clz.assert_called_once() open_mock.assert_has_calls([call('test1.conf', 'r'), call('test2.conf', 'r')], any_order=True) if six.PY2: - self.assertEqual(cp_clz.return_value.readfp.call_count, 2) + scp_clz.assert_called_once() + self.assertEqual(scp_clz.return_value.readfp.call_count, 2) else: + cp_clz.assert_called_once() self.assertEqual(cp_clz.return_value.read_file.call_count, 2) open_mock.reset_mock() cp_clz.reset_mock() - conf = koji.read_config_files(files, - six.moves.configparser.SafeConfigParser) + scp_clz.reset_mock() + conf = koji.read_config_files(files, raw=True) self.assertTrue(isinstance(conf, - six.moves.configparser.SafeConfigParser.__class__)) + six.moves.configparser.RawConfigParser.__class__)) cp_clz.assert_not_called() - scp_clz.assert_called_once() + scp_clz.assert_not_called() + rcp_clz.assert_called_once() class MavenUtilTestCase(unittest.TestCase): @@ -532,12 +539,13 @@ class MavenUtilTestCase(unittest.TestCase): self.assertEqual(cm.exception.args[0], 'total ordering not possible') def _read_conf(self, cfile): - config = six.moves.configparser.ConfigParser() path = os.path.dirname(__file__) with open(path + cfile, 'r') as conf_file: if six.PY2: + config = six.moves.configparser.SafeConfigParser() config.readfp(conf_file) else: + config = six.moves.configparser.ConfigParser() config.read_file(conf_file) return config diff --git a/tests/test_plugins/test_protonmsg.py b/tests/test_plugins/test_protonmsg.py index 5837891..bd466a4 100644 --- a/tests/test_plugins/test_protonmsg.py +++ b/tests/test_plugins/test_protonmsg.py @@ -9,7 +9,7 @@ except ImportError: from mock import patch, MagicMock from koji.context import context -from six.moves.configparser import SafeConfigParser +from six.moves.configparser import ConfigParser, SafeConfigParser class TestProtonMsg(unittest.TestCase): def tearDown(self): @@ -269,10 +269,11 @@ topic_prefix = koji connect_timeout = 10 send_timeout = 60 """) - conf = SafeConfigParser() if six.PY2: + conf = SafeConfigParser() conf.readfp(confdata) else: + conf = ConfigParser() conf.read_file(confdata) self.handler = protonmsg.TimeoutHandler('amqps://broker1.example.com:5671', [], conf) @@ -293,10 +294,11 @@ topic_prefix = koji connect_timeout = 10 send_timeout = 60 """) - conf = SafeConfigParser() if six.PY2: + conf = SafeConfigParser() conf.readfp(confdata) else: + conf = ConfigParser() conf.read_file(confdata) handler = protonmsg.TimeoutHandler('amqp://broker1.example.com:5672', [], conf) event = MagicMock()