[PATCH] Load config files from current directory by default

[Kamil Paral]

This patch ensures that the last remaining config file repoinfo.conf is
also loaded from current directory by default and only when it's not
present the system-wide location is used. It means that now all our
config files use CWD location by default and we don't have to maintain
/etc directory on AutoQA clients anymore.

It also adds a few comments to RepoinfoConfig object and repoinfo.conf.

Post-repo-update and post-tree-compose watchers have slightly modified
placeholder for architecture in repo URLs.

getbool() method now does not crash for non-string inputs.

Fixes ticket #253.
---
 hooks/post-repo-update/watch-repos.py     |   12 ++++++------
 hooks/post-tree-compose/watch-composes.py |   10 +++++-----
 lib/python/config.py                      |   16 +++++++++-------
 lib/python/repoinfo.py                    |   29 +++++++++++++++++++++++------
 repoinfo.conf                             |    4 ++++
 5 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/hooks/post-repo-update/watch-repos.py
b/hooks/post-repo-update/watch-repos.py
index 0d3ce8f..46bbb13 100755
--- a/hooks/post-repo-update/watch-repos.py
+++ b/hooks/post-repo-update/watch-repos.py
@@ -36,8 +36,8 @@ parser.add_option('--dryrun', '--dry-run',
action='store_true',
     help='Do not actually execute commands, just show what would be done')
 (opts, args) = parser.parse_args()
 
-# Set the default arch to our placeholder, '%a'
-repoinfo.setarch('%%a') # two %% because of ConfigParser interpolation
+# Set the default arch to our placeholder '$ARCH'
+repoinfo.setarch('$ARCH')
 
 # Setup a cache path
 cachedir = '/var/cache/autoqa/watch-repos'
@@ -65,7 +65,7 @@ for reponame in repoinfo.repos():
             cache = ''
         # grab repomd.xml from remote
         try:
-            urlpath = os.path.join(repo['url'].replace('%a',arch),
+            urlpath = os.path.join(repo['url'].replace('$ARCH',arch),
                                    'repodata', 'repomd.xml')
             remote = urlgrabber.urlread(urlpath)
         except Exception, e:
@@ -85,8 +85,8 @@ for reponame, arches in sorted(testable.items()):
                        '--arch', arch]
         # Add parent repos to the arglist
         for preponame in repo['parents']:
-            harnesscall += ['--parent', repoinfo.get(preponame,
'url').replace('%a', arch)]
-        harnesscall.append(repo['url'].replace('%a', arch))
+            harnesscall += ['--parent', repoinfo.get(preponame,
'url').replace('$ARCH', arch)]
+        harnesscall.append(repo['url'].replace('$ARCH', arch))
 
         if opts.dryrun:
             print ' '.join(harnesscall)
@@ -96,7 +96,7 @@ for reponame, arches in sorted(testable.items()):
         cachepath = os.path.join(cachedir, reponame, arch)
         if not os.path.exists(cachepath):
             os.makedirs(cachepath)
-        urlpath = os.path.join(repo['url'].replace('%a',arch),
+        urlpath = os.path.join(repo['url'].replace('$ARCH',arch),
                                'repodata',
                                'repomd.xml')
         output = os.path.join(cachepath, 'repomd.xml')
diff --git a/hooks/post-tree-compose/watch-composes.py
b/hooks/post-tree-compose/watch-composes.py
index a9d4e23..ca9c0d1 100755
--- a/hooks/post-tree-compose/watch-composes.py
+++ b/hooks/post-tree-compose/watch-composes.py
@@ -37,8 +37,8 @@ parser.add_option('--dryrun', '--dry-run',
action='store_true',
     help='Do not actually execute commands, just show what would be done')
 (opts, args) = parser.parse_args()
 
-# Set the default arch to our placeholder, '%a'
-repoinfo.setarch('%%a') # two %% because of ConfigParser interpolation
+# Set the default arch to our placeholder '$ARCH'
+repoinfo.setarch('$ARCH')
 
 # Get the list of repos to watch from repoinfo (see /etc/autoqa/repoinfo.conf)
 watchcomposes = [r for r in repoinfo.repos() if
getbool(repoinfo.get(r,'composes'))]
@@ -70,7 +70,7 @@ for reponame in watchcomposes:
         # check .treeinfo
         try:
             # XXX use urljoin?
-            urlpath = os.path.join(repo['url'].replace('%a',arch),
+            urlpath = os.path.join(repo['url'].replace('$ARCH',arch),
                                    '.treeinfo')
             remote = urlgrabber.urlread(urlpath)
         # XXX maybe less broad exception?
@@ -89,7 +89,7 @@ for reponame, arches in sorted(testable.items()):
         # construct arguments for call to test launcher
         harnesscall = ['autoqa', 'post-tree-compose', '--name',
repo['name'],
                        '--arch', arch]
-        harnesscall.append(repo['url'].replace('%a', arch))
+        harnesscall.append(repo['url'].replace('$ARCH', arch))
         if opts.dryrun:
             print ' '.join(harnesscall)
             continue
@@ -98,7 +98,7 @@ for reponame, arches in sorted(testable.items()):
         cachepath = os.path.join(cachedir, repo['path'], arch)
         if not os.path.exists(cachepath):
             os.makedirs(cachepath)
-        composepath = repo['url'].replace('%a', arch)
+        composepath = repo['url'].replace('$ARCH', arch)
         urlpath = os.path.join(composepath, '.treeinfo')
         output = os.path.join(cachepath, 'treeinfo')
         urlgrabber.urlgrab(urlpath, filename=output)
diff --git a/lib/python/config.py b/lib/python/config.py
index 955f7a0..c9a59ad 100644
--- a/lib/python/config.py
+++ b/lib/python/config.py
@@ -81,22 +81,24 @@ class SingleConfigParser(ConfigParser.SafeConfigParser):
         retval.update(items)
         return retval
 
-def getbool(string):
+def getbool(value):
     '''Converts a string into a boolean.
        Recognizes many usual strings: 1/0, yes/no, true/false, on/off; also
        empty strings.
        Throws ValueError if the string can't be recognized.
+       Uses default bool conversion for non-string values.
     '''
     boolean_states = {'1': True, 'yes': True,  'true': True,  
'on': True,
                       '0': False, 'no': False, 'false': False,
'off': False}
-    if not string:
-        return False
 
-    string = string.lower()
-    if string not in boolean_states:
-        raise ValueError, 'Not a boolean: %s' % string
+    if not isinstance(value, str):
+        return bool(value)
 
-    return boolean_states[string]
+    value = value.lower()
+    if value not in boolean_states:
+        raise ValueError, 'Not a boolean: %s' % value
+
+    return boolean_states[value]
 
 
 autoqa_conf = SingleConfigParser()
diff --git a/lib/python/repoinfo.py b/lib/python/repoinfo.py
index 3a8c1f8..de9f2de 100644
--- a/lib/python/repoinfo.py
+++ b/lib/python/repoinfo.py
@@ -20,24 +20,35 @@
 # Author: Will Woods <wwoods(a)redhat.com&gt;
 
 from ConfigParser import SafeConfigParser
+from config import SingleConfigParser
 import warnings
 
-configpath = ('/etc/autoqa/repoinfo.conf',
-              'repoinfo.conf')
+configpath = ('repoinfo.conf',
+              '/etc/autoqa/repoinfo.conf',)
 
 class RepoinfoConfig(object):
     def __init__(self, arch="$basearch", filelist=configpath, defaults={}):
+        '''Create RepoinfoConfig object.
+        Attributes:
+        * arch - architecture for which to adjust repo URLs
+        * filelist - list of config files to read information from, just the
+                     first available file will be used
+        * defaults - dictionary of default values
+        '''
         self.defaults = defaults
         self.filelist = filelist
         self.setarch(arch) # implies read()
 
     def _getlist(self, name, item):
+        '''Transform a list of comma separated values into a Python list
+        Returns empty list for non-existent item'''
         itemlist = self.config.get(name, item)
         if not itemlist:
             return []
         else:
             return [t.strip() for t in itemlist.split(',')]
     def getarches(self, name):
+        '''Get the list of all supported architectures for a
repo'''
         return self._getlist(name, 'arches')
     def getreleases(self):
         '''Return the list of known, non-EOL releases.'''
@@ -51,7 +62,8 @@ class RepoinfoConfig(object):
         return self._getlist(name, 'parents')
     def getrepo(self, name):
         '''Given a repo name, return the repoinfo dict, with the keys:
-        'arches', 'parents', 'tag', 'url',
'path', 'name' '''
+        'arches', 'parents', 'tag', 'url',
'path', 'name', 'collection_name'
+        and 'composes' '''
         repo = {'arches': self.getarches(name),
                 'parents': self.getparents(name),
                 'tag': self.config.get(name,'tag'),
@@ -59,7 +71,7 @@ class RepoinfoConfig(object):
                 'path': self.config.get(name,'path'),
                 'name': name, # kind of unnecessary
                 'collection_name': self.config.get(name,
'collection_name'),
-                'composes': self.config.get(name,'composes'),
+                'composes': self.config.getboolean(name,'composes'),
         }
         return repo
     def getrepo_by_tag(self, tag):
@@ -76,15 +88,20 @@ class RepoinfoConfig(object):
         '''Returns a list of all known repo names'''
         return self.config.sections()
     def setarch(self, arch):
+        '''Set architecture for which to adjust repo URLs'''
         self.defaults['arch'] = arch
         self.read()
     def read(self):
-        self.config = SafeConfigParser(self.defaults)
-        return self.config.read(self.filelist)
+        '''Apply the defaults and then read first available config file from
+        the list of provided config files.'''
+        self.config = SingleConfigParser(defaults=self.defaults)
+        self.config.read_single(self.filelist)
     def get(self, repo, key):
+        '''Get a specific key from a repo'''
         return self.config.get(repo, key)
 
 repoinfo = RepoinfoConfig()
+
 if not repoinfo.repos():
     warnings.warn("No repos found in (%s)" % ', '.join(configpath),
                   RuntimeWarning)
diff --git a/repoinfo.conf b/repoinfo.conf
index 4873302..6f5c279 100644
--- a/repoinfo.conf
+++ b/repoinfo.conf
@@ -1,6 +1,9 @@
 [DEFAULT]
+# list of parent repos to this repo
 parents =
+# list of supported architectures
 arches = i386, x86_64
+# whether installation composes are created for this repo or not
 composes =
 # tag defaults to dist-[section_name]
 tag = dist-%(__name__)s
@@ -8,6 +11,7 @@ baseurl = http://download.fedoraproject.org/pub/fedora/linux
 goldurl = %(baseurl)s/releases/%(path)s/Everything/%(arch)s/os
 updatesurl = %(baseurl)s/updates/%(path)s/%(arch)s
 rawhideurl = %(baseurl)s/%(path)s/%(arch)s/os
+# true for "main" repos corresponding to currently supported Fedora releases
 isactiverelease =
 
 # Rawhide
-- 
1.7.3.3

[James Laska]

On Tue, 2011-01-04 at 09:40 +0100, Kamil Páral wrote:
>  This patch ensures that the last remaining config file repoinfo.conf
> is
>  also loaded from current directory by default and only when it's not
>  present the system-wide location is used. It means that now all our
>  config files use CWD location by default and we don't have to maintain
>  /etc directory on AutoQA clients anymore.
>  
>  It also adds a few comments to RepoinfoConfig object and repoinfo.conf.
>  
>  Post-repo-update and post-tree-compose watchers have slightly modified
>  placeholder for architecture in repo URLs.
>  
>  getbool() method now does not crash for non-string inputs. 
Looks good and bonus points for adding docstrings :)

I've added a few comments below, otherwise ACK from me!

>  Fixes ticket #253.
>  ---
>   hooks/post-repo-update/watch-repos.py     |   12 ++++++------
>   hooks/post-tree-compose/watch-composes.py |   10 +++++-----
>   lib/python/config.py                      |   16 +++++++++------- 
No objections, looks like $ARCH is more self-documenting than %a.  I
wonder if we should be using string.format() instead of string.replace()
here?

http://docs.python.org/library/stdtypes.html#str.format

I'm not aware that it's better or worse than using string.replace(), I'm
just familiar with how Chris Lumens uses it for templating in the
anaconda_storage test.  Definitely your call on which is the appropriate
fit.

http://git.fedorahosted.org/git/?p=autoqa.git;a=blob;f=tests/anaconda_sto...

>   lib/python/repoinfo.py                    |   29
> +++++++++++++++++++++++------ 
ACK, that makes sense, and kudos for the docstring additions

>   repoinfo.conf                             |    4 ++++ 
Good comments, thanks.

>   5 files changed, 47 insertions(+), 24 deletions(-) 
Thanks,
James

[Kamil Paral]

----- Original Message -----
>  > ---
>  >  hooks/post-repo-update/watch-repos.py | 12 ++++++------
>  >  hooks/post-tree-compose/watch-composes.py | 10 +++++-----
>  >  lib/python/config.py | 16 +++++++++-------
>  
>  No objections, looks like $ARCH is more self-documenting than %a. I
>  wonder if we should be using string.format() instead of
>  string.replace()
>  here?
>  
>  http://docs.python.org/library/stdtypes.html#str.format
>  
>  I'm not aware that it's better or worse than using string.replace(),
>  I'm
>  just familiar with how Chris Lumens uses it for templating in the
>  anaconda_storage test. Definitely your call on which is the
>  appropriate
>  fit.
>  
>
> http://git.fedorahosted.org/git/?p=autoqa.git;a=blob;f=tests/anaconda_sto...
str.format seems to be available only from Python 2.6. Let's keep 
the str.replace() in.

>  
>  >  lib/python/repoinfo.py | 29 +++++++++++++++++++++++------
>  
>  ACK, that makes sense, and kudos for the docstring additions
>  
>  >  repoinfo.conf | 4 ++++
>  
>  Good comments, thanks.
>  
>  >  5 files changed, 47 insertions(+), 24 deletions(-)
>  
>  Thanks,
>  James 
Thank you for the review. Pushed to master.