#1296 extract read_config_files util for config parsing
Merged 2 years ago by tkopecek. Opened 3 years ago by julian8628.
julian8628/koji issue/865  into  master

file modified
+1 -3
@@ -47,7 +47,6 @@ 

  import traceback

  import xml.dom.minidom

  import zipfile

- from six.moves.configparser import RawConfigParser

  from fnmatch import fnmatch

  from gzip import GzipFile

  from optparse import OptionParser, SUPPRESS_HELP
@@ -6100,8 +6099,7 @@ 

          assert False  # pragma: no cover

  

      # load local config

-     config = RawConfigParser()

-     config.read(options.configFile)

+     config = koji.read_config_files(options.configFile, raw=True)

      for x in config.sections():

          if x != 'kojid':

              quit('invalid section found in config file: %s' % x)

file modified
+1 -3
@@ -5904,10 +5904,8 @@ 

      (task_options, args) = parser.parse_args(args)

  

      if task_options.config:

-         if not os.path.exists(task_options.config):

-             parser.error(_("%s not found!" % task_options.config))

          section = 'image-build'

-         config = koji.read_config_files(task_options.config)

+         config = koji.read_config_files([(task_options.config, True)])

          if not config.has_section(section):

              parser.error(_("single section called [%s] is required" % section))

          # pluck out the positional arguments first

file modified
+2 -12
@@ -20,7 +20,6 @@ 

  

  from __future__ import absolute_import

  from __future__ import division

- from six.moves.configparser import RawConfigParser

  import datetime

  import inspect

  import logging
@@ -396,17 +395,8 @@ 

      #get our config file(s)

      cf = environ.get('koji.hub.ConfigFile', '/etc/koji-hub/hub.conf')

      cfdir = environ.get('koji.hub.ConfigDir', '/etc/koji-hub/hub.conf.d')

-     if cfdir:

-         configs = koji.config_directory_contents(cfdir)

-     else:

-         configs = []

-     if cf and os.path.isfile(cf):

-         configs.append(cf)

-     if configs:

-         config = RawConfigParser()

-         config.read(configs)

-     else:

-         config = None

+     config = koji.read_config_files([cfdir, (cf, True)], raw=True)

+ 

      cfgmap = [

          #option, type, default

          ['DBName', 'string', None],

file modified
+85 -55
@@ -1673,7 +1673,7 @@ 

      return fo

  

  

- def config_directory_contents(dir_name):

+ def config_directory_contents(dir_name, strict=False):

      configs = []

      try:

          conf_dir_contents = os.listdir(dir_name)
@@ -1685,7 +1685,12 @@ 

              if not name.endswith('.conf'):

                  continue

              config_full_name = os.path.join(dir_name, name)

-             configs.append(config_full_name)

+             if os.path.isfile(config_full_name) \

+                and os.access(config_full_name, os.F_OK):

+                 configs.append(config_full_name)

+             elif strict:

+                 raise ConfigurationError("Config file %s can't be opened."

+                                          % config_full_name)

      return configs

  

  
@@ -1727,59 +1732,47 @@ 

      #note: later config files override earlier ones

  

      # /etc/koji.conf.d

-     configs = config_directory_contents('/etc/koji.conf.d')

+     configs = ['/etc/koji.conf.d']

  

      # /etc/koji.conf

-     if os.access('/etc/koji.conf', os.F_OK):

-         configs.append('/etc/koji.conf')

+     configs.append('/etc/koji.conf')

  

      # User specific configuration

      if user_config:

          # Config file specified on command line

-         fn = os.path.expanduser(user_config)

-         if os.path.isdir(fn):

-             # Specified config is a directory

-             contents = config_directory_contents(fn)

-             if not contents:

-                 raise ConfigurationError("No config files found in directory: %s" % fn)

-             configs.extend(contents)

-         else:

-             # Specified config is a file

-             if not os.access(fn, os.F_OK):

-                 raise ConfigurationError("No such file: %s" % fn)

-             configs.append(fn)

+         # The existence will be checked

+         configs.append((os.path.expanduser(user_config), True))

      else:

-         # User config

-         user_config_dir = os.path.expanduser("~/.koji/config.d")

-         configs.extend(config_directory_contents(user_config_dir))

-         fn = os.path.expanduser("~/.koji/config")

-         if os.access(fn, os.F_OK):

-             configs.append(fn)

+         # User config dir

+         configs.append(os.path.expanduser("~/.koji/config.d"))

+         # User config file

+         configs.append(os.path.expanduser("~/.koji/config"))

+ 

+     config = read_config_files(configs)

  

      # Load the configs in a particular order

      got_conf = False

-     for configFile in configs:

-         config = read_config_files(configFile)

-         if config.has_section(profile_name):

-             got_conf = True

-             for name, value in config.items(profile_name):

-                 #note the config_defaults dictionary also serves to indicate which

-                 #options *can* be set via the config file. Such options should

-                 #not have a default value set in the option parser.

-                 if name in result:

-                     if name in ('anon_retry', 'offline_retry', 'use_fast_upload',

-                                 'krb_rdns', 'debug', 'debug', 'debug_xmlrpc', 'krb_canon_host'):

-                         result[name] = config.getboolean(profile_name, name)

-                     elif name in ('max_retries', 'retry_interval',

-                                   'offline_retry_interval', 'poll_interval',

-                                   'timeout', 'auth_timeout',

-                                   'upload_blocksize', 'pyver'):

-                         try:

-                             result[name] = int(value)

-                         except ValueError:

-                             raise ConfigurationError("value for %s config option must be a valid integer" % name)

-                     else:

-                         result[name] = value

+     if config.has_section(profile_name):

+         got_conf = True

+         for name, value in config.items(profile_name):

+             #note the config_defaults dictionary also serves to indicate which

+             #options *can* be set via the config file. Such options should

+             #not have a default value set in the option parser.

+             if name in result:

+                 if name in ('anon_retry', 'offline_retry',

+                             'use_fast_upload', 'krb_rdns', 'debug',

+                             'debug_xmlrpc', 'krb_canon_host'):

+                     result[name] = config.getboolean(profile_name, name)

+                 elif name in ('max_retries', 'retry_interval',

+                               'offline_retry_interval', 'poll_interval',

+                               'timeout', 'auth_timeout',

+                               'upload_blocksize', 'pyver'):

+                     try:

+                         result[name] = int(value)

+                     except ValueError:

+                         raise ConfigurationError("value for %s config option must be a valid integer" % name)

+                 else:

+                     result[name] = value

  

      # Check if the specified profile had a config specified

      if configs and not got_conf:
@@ -1855,14 +1848,41 @@ 

  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 config_files: config file(s) to read (required). Config file could

+                          be file or directory, and order is preserved.

+                          If it's a list/tuple of list/tuple, in each inner

+                          item, the 1st item is file/dir name, and the 2nd item

+                          is strict(False if not present), which indicate

+                          if checking that:

+                              1. is dir an empty directory

+                              2. dose file exist

+                              3. is file accessible

+                          raising ConfigurationError if any above is True.

+     :type config_files: str or list or tuple

      :param bool raw: enable 'raw' parsing (no interpolation). Default: False

  

      :return: object of parser which contains parsed content

+ 

+     :raises: GenericError: config_files is not valid

+              ConfigurationError: See config_files if strict is true

+              OSError: Directory in config_files is not accessible

      """

-     if not isinstance(config_files, (list, tuple)):

-         config_files = [config_files]

+     if isinstance(config_files, six.string_types):

+         config_files = [(config_files, False)]

+     elif isinstance(config_files, (list, tuple)):

+         fcfgs = []

+         for i in config_files:

+             if isinstance(i, six.string_types):

+                 fcfgs.append((i, False))

+             elif isinstance(i, (list, tuple)) and 0 < len(i) <= 2:

+                 fcfgs.append((i[0], i[1] if len(i) == 2 else False))

+             else:

+                 raise GenericError('invalid value: %s or type: %s'

+                                    % (i, type(i)))

+         config_files = fcfgs

+     else:

+         raise GenericError('invalid type: %s' % type(config_files))

+ 

      if raw:

          parser = six.moves.configparser.RawConfigParser

      elif six.PY2:
@@ -1872,12 +1892,22 @@ 

          # deprecated alias

          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)

+     cfgs = []

+     # append dir contents

+     for config_file, strict in config_files:

+         if os.path.isdir(config_file):

+             fns = config_directory_contents(config_file, strict=strict)

+             if fns:

+                 cfgs.extend(fns)

+             elif strict:

+                 raise ConfigurationError("No config files found in directory:"

+                                          " %s" % config_file)

+         elif os.path.isfile(config_file) and os.access(config_file, os.F_OK):

+             cfgs.append(config_file)

+         elif strict:

+             raise ConfigurationError("Config file %s can't be opened."

+                                      % config_file)

+     config.read(cfgs)

      return config

  

  

file modified
+1 -2
@@ -49,8 +49,7 @@ 

          return res

  

      def _read_config(self):

-         cp = six.moves.configparser.SafeConfigParser()

-         cp.read(CONFIG_FILE)

+         cp = koji.read_config_files(CONFIG_FILE)

          self.config = {

             'default_mounts': [],

             'safe_roots': [],

@@ -3,7 +3,6 @@ 

  import os

  import sys

  import tarfile

- import six.moves.configparser

  

  import koji

  import koji.tasks as tasks
@@ -28,8 +27,7 @@ 

  

  def read_config():

      global config

-     cp = six.moves.configparser.SafeConfigParser()

-     cp.read(CONFIG_FILE)

+     cp = koji.read_config_files(CONFIG_FILE)

      config = {

          'path_filters': [],

          'volume': None,

file modified
+1 -1
@@ -269,7 +269,7 @@ 

      log = logging.getLogger('koji.plugin.protonmsg')

      global CONFIG

      if not CONFIG:

-         CONFIG = koji.read_config_files(CONFIG_FILE)

+         CONFIG = koji.read_config_files([(CONFIG_FILE, True)])

      urls = CONFIG.get('broker', 'urls').split()

      test_mode = False

      if CONFIG.has_option('broker', 'test_mode'):

file modified
+4 -4
@@ -10,16 +10,15 @@ 

  from koji.context import context

  from koji.plugin import callback

  from koji.util import rmtree

- import six.moves.configparser

  import fnmatch

  import os

- import shutil

  import subprocess

  

  CONFIG_FILE = '/etc/koji-hub/plugins/rpm2maven.conf'

  

  config = None

  

+ 

  @callback('postImport')

  def maven_import(cbtype, *args, **kws):

      global config
@@ -32,8 +31,7 @@ 

      filepath = kws['filepath']

  

      if not config:

-         config = six.moves.configparser.SafeConfigParser()

-         config.read(CONFIG_FILE)

+         config = koji.read_config_files([(CONFIG_FILE, True)])

      name_patterns = config.get('patterns', 'rpm_names').split()

      for pattern in name_patterns:

          if fnmatch.fnmatch(rpminfo['name'], pattern):
@@ -52,6 +50,7 @@ 

          if os.path.exists(tmpdir):

              rmtree(tmpdir)

  

+ 

  def expand_rpm(filepath, tmpdir):

      devnull = open('/dev/null', 'r+')

      rpm2cpio = subprocess.Popen(['/usr/bin/rpm2cpio', filepath],
@@ -69,6 +68,7 @@ 

                (filepath, rpm2cpio.wait(), cpio.wait()))

      devnull.close()

  

+ 

  def scan_and_import(buildinfo, rpminfo, tmpdir):

      global config

      path_patterns = config.get('patterns', 'artifact_paths').split()

@@ -1,6 +1,5 @@ 

  from __future__ import absolute_import

  import sys

- import six.moves.configparser

  import koji

  from koji.context import context

  from koji.plugin import export
@@ -29,8 +28,7 @@ 

  

      # read configuration only once

      if config is None:

-         config = six.moves.configparser.SafeConfigParser()

-         config.read(CONFIG_FILE)

+         config = koji.read_config_files([(CONFIG_FILE, True)])

          allowed_methods = config.get('permissions', 'allowed_methods').split()

          if len(allowed_methods) == 1 and allowed_methods[0] == '*':

              allowed_methods = '*'

@@ -0,0 +1,1 @@ 

+ # empty config file 

\ No newline at end of file

@@ -12,7 +12,10 @@ 

  from koji_cli.commands import handle_image_build, _build_image_oz

  from . import utils

  

- ConfigParser = six.moves.configparser.ConfigParser

+ if six.PY2:

+     ConfigParser = six.moves.configparser.SafeConfigParser

+ else:

+     ConfigParser = six.moves.configparser.ConfigParser

  

  TASK_OPTIONS = {

      "background": None,
@@ -240,28 +243,32 @@ 

          """Test handle_image_build argument with --config cases"""

  

          # Case 1, config file not exist case

-         self.assert_system_exit(

-                 handle_image_build,

-                 self.options,

-                 self.session,

-                 ['--config', '/nonexistent-file-755684354'],

-                 stderr=self.format_error_message('/nonexistent-file-755684354 not found!'),

-                 activate_session=None)

+         with self.assertRaises(koji.ConfigurationError) as cm:

+             handle_image_build(self.options,

+                                self.session,

+                                ['--config', '/nonexistent-file-755684354'])

+         self.assertEqual(cm.exception.args[0],

+                          "Config file /nonexistent-file-755684354 can't be opened.")

  

-         # Case 2, no image-build section in config file

  

+         # Case 2, no image-build section in config file

          expected = "single section called [%s] is required" % "image-build"

  

          self.configparser.return_value = ConfigParser()

+ 

          self.assert_system_exit(

              handle_image_build,

              self.options,

              self.session,

-             ['--config', '/dev/null'],

+             ['--config',

+              os.path.join(os.path.dirname(__file__),

+                           'data/image-build-config-empty.conf')],

              stderr=self.format_error_message(expected),

              activate_session=None)

  

-         config_file = os.path.join(os.path.dirname(__file__), 'data/image-build-config.conf')

+         config_file = os.path.join(os.path.dirname(__file__),

+                                    'data/image-build-config.conf')

+         # Case 3, normal

          handle_image_build(

              self.options,

              self.session,

file modified
+150 -28
@@ -162,49 +162,171 @@ 

  class ConfigFileTestCase(unittest.TestCase):

      """Test config file reading functions"""

  

-     @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, rcp_clz, cp_clz, open_mock):

+     def setUp(self):

+         self.manager = mock.MagicMock()

+         self.manager.logging = mock.patch('koji.logging').start()

+         self.manager.isdir = mock.patch("os.path.isdir").start()

+         self.manager.isfile = mock.patch("os.path.isfile").start()

+         self.manager.access = mock.patch("os.access", return_value=True).start()

+         self.manager.cp_clz = mock.patch("six.moves.configparser.ConfigParser",

+                                          spec=True).start()

+         self.manager.scp_clz = mock.patch("six.moves.configparser.SafeConfigParser",

+                                          spec=True).start()

+         self.manager.rcp_clz = mock.patch("six.moves.configparser.RawConfigParser",

+                                          spec=True).start()

+         if six.PY2:

+             self.real_parser_clz = self.manager.scp_clz

+         else:

+             self.real_parser_clz = self.manager.cp_clz

+         self.mocks = [self.manager.isdir,

+                       self.manager.isfile,

+                       self.manager.access,

+                       self.manager.open,

+                       self.manager.cp_clz,

+                       self.manager.scp_clz,

+                       self.manager.rcp_clz]

+ 

+     def reset_mock(self):

+         for m in self.mocks:

+             m.reset_mock()

+ 

+     def tearDown(self):

+         mock.patch.stopall()

+ 

+     def test_read_config_files(self):

+ 

+         # bad config_files

+         for files in [0,

+                       False,

+                       set(),

+                       dict(),

+                       object(),

+                       ('string', True),

+                       [('str', True, 'str')],

+                       [tuple()],]:

+             with self.assertRaises(koji.GenericError):

+                 koji.read_config_files(files)

+ 

+         # string as config_files

          files = 'test1.conf'

+         self.manager.isdir.return_value = False

          conf = koji.read_config_files(files)

-         open_mock.assert_called_once_with(files, 'r')

+         self.manager.isdir.assert_called_once_with(files)

          if six.PY2:

              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()

+         self.real_parser_clz.assert_called_once()

+         self.real_parser_clz.return_value.read.assert_called_once_with([files])

  

-         open_mock.reset_mock()

-         cp_clz.reset_mock()

-         scp_clz.reset_mock()

+         # list as config_files

+         self.reset_mock()

          files = ['test1.conf', 'test2.conf']

          koji.read_config_files(files)

-         open_mock.assert_has_calls([call('test1.conf', 'r'),

-                                     call('test2.conf', 'r')],

-                                    any_order=True)

-         if six.PY2:

-             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()

-         scp_clz.reset_mock()

+         self.real_parser_clz.assert_called_once()

+         self.real_parser_clz.return_value.read.assert_called_once_with(files)

+ 

+         # tuple as config_files

+         self.reset_mock()

+         files = ('test1.conf', 'test2.conf')

+         koji.read_config_files(files)

+ 

+         # raw

+         self.reset_mock()

          conf = koji.read_config_files(files, raw=True)

          self.assertTrue(isinstance(conf,

                                     six.moves.configparser.RawConfigParser.__class__))

-         cp_clz.assert_not_called()

-         scp_clz.assert_not_called()

-         rcp_clz.assert_called_once()

+         self.manager.cp_clz.assert_not_called()

+         self.manager.scp_clz.assert_not_called()

+         self.manager.rcp_clz.assert_called_once()

+ 

+         # strict

+         # case1, not a file

+         self.reset_mock()

+         files = [('test1.conf',), ('test2.conf', True)]

+         self.manager.isfile.return_value = False

+         with self.assertRaises(koji.ConfigurationError) as cm:

+             koji.read_config_files(files)

+         self.assertEqual(cm.exception.args[0],

+                          "Config file test2.conf can't be opened.")

+ 

+         self.assertEqual(self.manager.isdir.call_count, 2)

+         self.assertEqual(self.manager.isfile.call_count, 2)

+         self.manager.access.assert_not_called()

+ 

+         # case2, inaccessible

+         self.reset_mock()

+         self.manager.isfile.return_value = True

+         self.manager.access.return_value = False

+         with self.assertRaises(koji.ConfigurationError) as cm:

+             koji.read_config_files(files)

+         self.assertEqual(cm.exception.args[0],

+                          "Config file test2.conf can't be opened.")

+         self.assertEqual(self.manager.isdir.call_count, 2)

+         self.assertEqual(self.manager.isfile.call_count, 2)

+         self.assertEqual(self.manager.access.call_count, 2)

+ 

+         # directories

+         # strict==False

+         self.reset_mock()

+         files = ['test1.conf', 'gooddir', 'test2.conf', 'emptydir', 'nonexistdir']

+         self.manager.isdir.side_effect = lambda f: False \

+             if f in ['test1.conf', 'test2.conf', 'nonexistdir'] else True

+         self.manager.isfile.side_effect = lambda f: False \

+             if f in ['nonexistdir', 'gooddir/test1-4.dir.conf'] else True

+         self.manager.access.return_value = True

+         with mock.patch("os.listdir", side_effect=[['test1-2.conf',

+                                                     'test1-1.conf',

+                                                     'test1-3.txt',

+                                                     'test1-4.dir.conf'],

+                                                    []]) as listdir_mock:

+             conf = koji.read_config_files(files)

+         listdir_mock.assert_has_calls([call('gooddir'), call('emptydir')])

+         self.real_parser_clz.assert_called_once()

+         self.real_parser_clz.return_value.read.assert_called_once_with(

+             ['test1.conf',

+              'gooddir/test1-1.conf',

+              'gooddir/test1-2.conf',

+              'test2.conf'])

+         self.assertEqual(self.manager.isdir.call_count, 5)

+         self.assertEqual(self.manager.isfile.call_count, 6)

+         self.assertEqual(self.manager.access.call_count, 4)

+ 

+         # strict==True

+         # case1

+         self.reset_mock()

+         files[1] = ('gooddir', True)

+         with mock.patch("os.listdir", return_value=['test1-2.conf',

+                                                     'test1-1.conf',

+                                                     'test1-3.txt',

+                                                     'test1-4.dir.conf']

+                         ) as listdir_mock:

+             with self.assertRaises(koji.ConfigurationError) as cm:

+                 conf = koji.read_config_files(files)

+         self.assertEqual(cm.exception.args[0],

+                          "Config file gooddir/test1-4.dir.conf can't be"

+                          " opened.")

+         listdir_mock.assert_called_once_with('gooddir')

+ 

+         # case2

+         self.reset_mock()

+         files[1] = ('gooddir', False)

+         files[3] = ('emptydir', True)

+         with mock.patch("os.listdir", side_effect=[['test1-2.conf',

+                                                     'test1-1.conf',

+                                                     'test1-3.txt',

+                                                     'test1-4.dir.conf'],

+                                                    []]

+                         ) as listdir_mock:

+             with self.assertRaises(koji.ConfigurationError) as cm:

+                 conf = koji.read_config_files(files)

+         self.assertEqual(cm.exception.args[0],

+                          'No config files found in directory: emptydir')

+         self.assertEqual(listdir_mock.call_count, 2)

+ 

  

  

  class MavenUtilTestCase(unittest.TestCase):

@@ -24,6 +24,12 @@ 

          return mock.patch('builtins.open')

  

  

+ if six.PY2:

+     CONFIG_PARSER = 'six.moves.configparser.SafeConfigParser'

+ else:

+     CONFIG_PARSER = 'six.moves.configparser.ConfigParser'

+ 

+ 

  CONFIG1 = {

          'paths': {

              'default_mounts': '/mnt/archive,/mnt/workdir',
@@ -96,11 +102,11 @@ 

  

  

  class TestRunrootConfig(unittest.TestCase):

-     @mock.patch('six.moves.configparser.SafeConfigParser')

-     def test_bad_config_paths0(self, safe_config_parser):

+     @mock.patch(CONFIG_PARSER)

+     def test_bad_config_paths0(self, config_parser):

          cp = FakeConfigParser()

          del cp.CONFIG['path0']['mountpoint']

-         safe_config_parser.return_value = cp

+         config_parser.return_value = cp

          session = mock.MagicMock()

          options = mock.MagicMock()

          options.workdir = '/tmp/nonexistentdirectory'
@@ -109,11 +115,11 @@ 

          self.assertEqual(cm.exception.args[0],

              "bad config: missing options in path0 section")

  

-     @mock.patch('six.moves.configparser.SafeConfigParser')

-     def test_bad_config_absolute_path(self, safe_config_parser):

+     @mock.patch(CONFIG_PARSER)

+     def test_bad_config_absolute_path(self, config_parser):

          cp = FakeConfigParser()

          cp.CONFIG['paths']['default_mounts'] = ''

-         safe_config_parser.return_value = cp

+         config_parser.return_value = cp

          session = mock.MagicMock()

          options = mock.MagicMock()

          options.workdir = '/tmp/nonexistentdirectory'
@@ -122,29 +128,29 @@ 

          self.assertEqual(cm.exception.args[0],

              "bad config: all paths (default_mounts, safe_roots, path_subs) needs to be absolute: ")

  

-     @mock.patch('six.moves.configparser.SafeConfigParser')

-     def test_valid_config(self, safe_config_parser):

-         safe_config_parser.return_value = FakeConfigParser()

+     @mock.patch(CONFIG_PARSER)

+     def test_valid_config(self, config_parser):

+         config_parser.return_value = FakeConfigParser()

          session = mock.MagicMock()

          options = mock.MagicMock()

          options.workdir = '/tmp/nonexistentdirectory'

          runroot.RunRootTask(123, 'runroot', {}, session, options)

  

-     @mock.patch('six.moves.configparser.SafeConfigParser')

-     def test_valid_config_alt(self, safe_config_parser):

-         safe_config_parser.return_value = FakeConfigParser(CONFIG2)

+     @mock.patch(CONFIG_PARSER)

+     def test_valid_config_alt(self, config_parser):

+         config_parser.return_value = FakeConfigParser(CONFIG2)

          session = mock.MagicMock()

          options = mock.MagicMock()

          options.workdir = '/tmp/nonexistentdirectory'

          runroot.RunRootTask(123, 'runroot', {}, session, options)

  

-     @mock.patch('six.moves.configparser.SafeConfigParser')

-     def test_pathnum_gaps(self, safe_config_parser):

+     @mock.patch(CONFIG_PARSER)

+     def test_pathnum_gaps(self, config_parser):

          session = mock.MagicMock()

          options = mock.MagicMock()

          options.workdir = '/tmp/nonexistentdirectory'

          config = CONFIG2.copy()

-         safe_config_parser.return_value = FakeConfigParser(config)

+         config_parser.return_value = FakeConfigParser(config)

          task1 = runroot.RunRootTask(123, 'runroot', {}, session, options)

          # adjust the path numbers (but preserving order) and do it again

          config = CONFIG2.copy()
@@ -152,29 +158,29 @@ 

          config['path999'] = config['path2']

          del config['path1']

          del config['path2']

-         safe_config_parser.return_value = FakeConfigParser(config)

+         config_parser.return_value = FakeConfigParser(config)

          task2 = runroot.RunRootTask(123, 'runroot', {}, session, options)

          # resulting processed config should be the same

          self.assertEqual(task1.config, task2.config)

          paths = list([CONFIG2[k] for k in ('path0', 'path1', 'path2')])

          self.assertEqual(task2.config['paths'], paths)

  

-     @mock.patch('six.moves.configparser.SafeConfigParser')

-     def test_bad_path_sub(self, safe_config_parser):

+     @mock.patch(CONFIG_PARSER)

+     def test_bad_path_sub(self, config_parser):

          session = mock.MagicMock()

          options = mock.MagicMock()

          options.workdir = '/tmp/nonexistentdirectory'

          config = copy.deepcopy(CONFIG2)

          config['paths']['path_subs'] += 'incorrect:format'

-         safe_config_parser.return_value = FakeConfigParser(config)

+         config_parser.return_value = FakeConfigParser(config)

          with self.assertRaises(koji.GenericError):

              runroot.RunRootTask(123, 'runroot', {}, session, options)

  

  

  class TestMounts(unittest.TestCase):

-     @mock.patch('six.moves.configparser.SafeConfigParser')

-     def setUp(self, safe_config_parser):

-         safe_config_parser.return_value = FakeConfigParser()

+     @mock.patch(CONFIG_PARSER)

+     def setUp(self, config_parser):

+         config_parser.return_value = FakeConfigParser()

          self.session = mock.MagicMock()

          options = mock.MagicMock()

          options.workdir = '/tmp/nonexistentdirectory'
@@ -324,8 +330,8 @@ 

          os_unlink.assert_not_called()

  

  class TestHandler(unittest.TestCase):

-     @mock.patch('six.moves.configparser.SafeConfigParser')

-     def setUp(self, safe_config_parser):

+     @mock.patch(CONFIG_PARSER)

+     def setUp(self, config_parser):

          self.session = mock.MagicMock()

          self.br = mock.MagicMock()

          self.br.mock.return_value = 0

file modified
+46 -58
@@ -108,62 +108,50 @@ 

  

      defaults = parser.get_default_values()

  

-     config = six.moves.configparser.ConfigParser()

-     cf = getattr(options, 'config_file', None)

-     if cf:

-         if not os.access(cf, os.F_OK):

-             parser.error(_("No such file: %s") % cf)

-             assert False  # pragma: no cover

-     else:

-         cf = '/etc/koji-gc/koji-gc.conf'

-         if not os.access(cf, os.F_OK):

-             cf = None

-     if not cf:

-         print("no config file")

-         config = None

-     else:

-         config.read(cf)

-         # List of values read from config file to update default parser values

-         cfgmap = [

-             # name, alias, type

-             ['keytab', None, 'string'],

-             ['principal', None, 'string'],

-             ['krbservice', None, 'string'],

-             ['krb_rdns', None, 'boolean'],

-             ['krb_canon_host', None, 'boolean'],

-             ['runas', None, 'string'],

-             ['user', None, 'string'],

-             ['password', None, 'string'],

-             ['noauth', None, 'boolean'],

-             ['cert', None, 'string'],

-             ['ca', None, 'string'],  # FIXME: remove in next major release

-             ['serverca', None, 'string'],

-             ['server', None, 'string'],

-             ['weburl', None, 'string'],

-             ['smtp_host', None, 'string'],

-             ['from_addr', None, 'string'],

-             ['email_template', None, 'string'],

-             ['email_domain', None, 'string'],

-             ['mail', None, 'boolean'],

-             ['delay', None, 'string'],

-             ['unprotected_keys', None, 'string'],

-             ['grace_period', None, 'string'],

-             ['trashcan_tag', None, 'string'],

-             ['no_ssl_verify', None, 'boolean'],

-             ['timeout', None, 'integer'],

-             ]

-         for name, alias, type in cfgmap:

-             if alias is None:

-                 alias = ('main', name)

-             if config.has_option(*alias):

-                 if options.debug:

-                     print("Using option %s from config file" % (alias,))

-                 if type == 'integer':

-                     setattr(defaults, name, config.getint(*alias))

-                 elif type == 'boolean':

-                     setattr(defaults, name, config.getboolean(*alias))

-                 else:

-                     setattr(defaults, name, config.get(*alias))

+     cf = getattr(options, 'config_file', '/etc/koji-gc/koji-gc.conf')

+     config = koji.read_config_files(cf)

+ 

+     # List of values read from config file to update default parser values

+     cfgmap = [

+         # name, alias, type

+         ['keytab', None, 'string'],

+         ['principal', None, 'string'],

+         ['krbservice', None, 'string'],

+         ['krb_rdns', None, 'boolean'],

+         ['krb_canon_host', None, 'boolean'],

+         ['runas', None, 'string'],

+         ['user', None, 'string'],

+         ['password', None, 'string'],

+         ['noauth', None, 'boolean'],

+         ['cert', None, 'string'],

+         ['ca', None, 'string'],  # FIXME: remove in next major release

+         ['serverca', None, 'string'],

+         ['server', None, 'string'],

+         ['weburl', None, 'string'],

+         ['smtp_host', None, 'string'],

+         ['from_addr', None, 'string'],

+         ['email_template', None, 'string'],

+         ['email_domain', None, 'string'],

+         ['mail', None, 'boolean'],

+         ['delay', None, 'string'],

+         ['unprotected_keys', None, 'string'],

+         ['grace_period', None, 'string'],

+         ['trashcan_tag', None, 'string'],

+         ['no_ssl_verify', None, 'boolean'],

+         ['timeout', None, 'integer'],

+         ]

+     for name, alias, type in cfgmap:

+         if alias is None:

+             alias = ('main', name)

+         if config.has_option(*alias):

+             if options.debug:

+                 print("Using option %s from config file" % (alias,))

+             if type == 'integer':

+                 setattr(defaults, name, config.getint(*alias))

+             elif type == 'boolean':

+                 setattr(defaults, name, config.getboolean(*alias))

+             else:

+                 setattr(defaults, name, config.get(*alias))

      #parse again with defaults

      (options, args) = parser.parse_args(values=defaults)

      options.config = config
@@ -187,8 +175,8 @@ 

      #parse key aliases

      options.key_aliases = {}

      try:

-         if config and config.has_option('main', 'key_aliases'):

-             for line in config.get('main','key_aliases').splitlines():

+         if config.has_option('main', 'key_aliases'):

+             for line in config.get('main', 'key_aliases').splitlines():

                  parts = line.split()

                  if len(parts) < 2:

                      continue

file modified
+38 -51
@@ -31,7 +31,6 @@ 

      krbV = None

  import koji

  from koji.util import to_list

- import six.moves.configparser

  import fnmatch

  import optparse

  import os
@@ -154,56 +153,44 @@ 

      (options, args) = parser.parse_args()

  

      defaults = parser.get_default_values()

-     config = six.moves.configparser.ConfigParser()

-     cf = getattr(options, 'config_file', None)

-     if cf:

-         if not os.access(cf, os.F_OK):

-             parser.error(_("No such file: %s") % cf)

-             assert False  # pragma: no cover

-     else:

-         cf = '/etc/koji-shadow/koji-shadow.conf'

-         if not os.access(cf, os.F_OK):

-             cf = None

-     if not cf:

-         log("no config file")

-         config = None

-     else:

-         config.read(cf)

-         #allow config file to update defaults

-         for opt in parser.option_list:

-             if not opt.dest:

-                 continue

-             name = opt.dest

-             alias = ('main', name)

-             if config.has_option(*alias):

-                 log("Using option %s from config file" % (alias,))

-                 if opt.action in ('store_true', 'store_false'):

-                     setattr(defaults, name, config.getboolean(*alias))

-                 elif opt.action != 'store':

-                     pass

-                 elif opt.type in ('int', 'long'):

-                     setattr(defaults, name, config.getint(*alias))

-                 elif opt.type in ('float'):

-                     setattr(defaults, name, config.getfloat(*alias))

-                 else:

-                     log(config.get(*alias))

-                     setattr(defaults, name, config.get(*alias))

-         #config file options without a cmdline equivalent

-         otheropts = [

-             #name, type, default

-             ['keytab', None, 'string'],

-             ['principal', None, 'string'],

-             ['runas', None, 'string'],

-             ['user', None, 'string'],

-             ['password', None, 'string'],

-             ['noauth', None, 'boolean'],

-             ['server', None, 'string'],

-             ['remote', None, 'string'],

-             ['max_jobs', None, 'int'],

-             ['serverca', None, 'string'],

-             ['auth_cert', None, 'string'],

-             ['arches', None, 'string'],

-             ]

+     cf = getattr(options, 'config_file', '/etc/koji-shadow/koji-shadow.conf')

+     config = koji.read_config_files(cf)

+ 

+     #allow config file to update defaults

+     for opt in parser.option_list:

+         if not opt.dest:

+             continue

+         name = opt.dest

+         alias = ('main', name)

+         if config.has_option(*alias):

+             log("Using option %s from config file" % (alias,))

+             if opt.action in ('store_true', 'store_false'):

+                 setattr(defaults, name, config.getboolean(*alias))

+             elif opt.action != 'store':

+                 pass

+             elif opt.type in ('int', 'long'):

+                 setattr(defaults, name, config.getint(*alias))

+             elif opt.type in ('float'):

+                 setattr(defaults, name, config.getfloat(*alias))

+             else:

+                 log(config.get(*alias))

+                 setattr(defaults, name, config.get(*alias))

+     #config file options without a cmdline equivalent

+     otheropts = [

+         #name, type, default

+         ['keytab', None, 'string'],

+         ['principal', None, 'string'],

+         ['runas', None, 'string'],

+         ['user', None, 'string'],

+         ['password', None, 'string'],

+         ['noauth', None, 'boolean'],

+         ['server', None, 'string'],

+         ['remote', None, 'string'],

+         ['max_jobs', None, 'int'],

+         ['serverca', None, 'string'],

+         ['auth_cert', None, 'string'],

+         ['arches', None, 'string'],

+         ]

  

  

      #parse again with updated defaults

file modified
+1 -3
@@ -27,7 +27,6 @@ 

  import koji

  from koji.util import rmtree, parseStatus, to_list

  from optparse import OptionParser

- from six.moves.configparser import ConfigParser

  import errno

  import json

  import logging
@@ -901,8 +900,7 @@ 

      parser.add_option("--logfile", help="Specify logfile")

      (options, args) = parser.parse_args()

  

-     config = ConfigParser()

-     config.read(options.configFile)

+     config = koji.read_config_files(options.configFile)

      section = 'kojira'

      for x in config.sections():

          if x != section:

file modified
+5 -2
@@ -28,7 +28,7 @@ 

  

  from __future__ import absolute_import

  from optparse import OptionParser

- from six.moves.configparser import ConfigParser

+ from six.moves.configparser import ConfigParser, SafeConfigParser

  import os

  import subprocess

  import sys
@@ -212,7 +212,10 @@ 

          elif len(specfiles) > 1:

              raise BuildError('Multiple .ini files found')

  

-         conf = ConfigParser()

+         if six.PY2:

+             conf = SafeConfigParser()

+         else:

+             conf = ConfigParser()

          conf.read(os.path.join(self.spec_dir, specfiles[0]))

  

          # [naming] section

file modified
+1 -3
@@ -46,7 +46,6 @@ 

  import pwd

  import requests

  import fnmatch

- from six.moves.configparser import ConfigParser

  from contextlib import closing

  from optparse import OptionParser

  try:
@@ -102,8 +101,7 @@ 

          assert False  # pragma: no cover

  

      # load local config

-     config = ConfigParser()

-     config.read(options.configFile)

+     config = koji.read_config_files(options.configFile)

      for x in config.sections():

          if x != 'kojivmd':

              quit('invalid section found in config file: %s' % x)

file modified
+2 -13
@@ -30,7 +30,6 @@ 

  import sys

  import traceback

  

- from six.moves.configparser import RawConfigParser

  from koji.server import ServerError, ServerRedirect

  from koji.util import dslice, to_list

  import six
@@ -127,22 +126,12 @@ 

          """

          cf = environ.get('koji.web.ConfigFile', '/etc/kojiweb/web.conf')

          cfdir = environ.get('koji.web.ConfigDir', '/etc/kojiweb/web.conf.d')

-         if cfdir:

-             configs = koji.config_directory_contents(cfdir)

-         else:

-             configs = []

-         if cf and os.path.isfile(cf):

-             configs.append(cf)

-         if configs:

-             config = RawConfigParser()

-             config.read(configs)

-         else:

-             raise koji.GenericError("Configuration missing")

+         config = koji.read_config_files([cfdir, (cf, True)])

  

          opts = {}

          for name, dtype, default in self.cfgmap:

              key = ('web', name)

-             if config and config.has_option(*key):

+             if config.has_option(*key):

                  if dtype == 'integer':

                      opts[name] = config.getint(*key)

                  elif dtype == 'boolean':

fixes: #865
based on PR#866 and PR#1150

  • extend read_config_files with strict option and directory support
  • use ConfigParser for all invokings except kojixmlrpc

rebased onto 7ea77b549e969fdb6f2178c9319cac6f543c0c94

2 years ago

1 new commit added

  • fix test_runroot_builder tests
2 years ago

change ConfigParser to RawConfigParser for kojid, related to PR #1543

cc @tkopecek

rebased onto 8d5525e47b9dc7108d2c7aa6c77473694a09c90a

2 years ago

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 years ago

rebased onto 8120e03

2 years ago

add some tweaks
- remove default argument. The first config in config_files is actually the default one
- drop strict option for all config files and dirs
- change config_files argument, it could be a list of string and tuple, the second item in the tuple is actualyl strict option for this config file/dir(the 1st item).

In previous code, "global" strict option is too strict.

Commit c05d8ea fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago