3
\$\begingroup\$

I've never done anything like this before so I'd like someone else to look at this before I get too carried away :)

Am I making this more complicated than it needs to be? I'm trying to make it EASY for other modules/scripts on this system to store and retrieve their settings. Hence why I trap the ConfigParser.NoOptionError error and return None and create the section if it doesn't exist in the set() method.

Suggestions?

import ConfigParser
import os

from ast import literal_eval as Eval

class _ConfParse(ConfigParser.ConfigParser):
    def __init__(self, confpath, conffile):
        ConfigParser.ConfigParser.__init__(self)
        self.conf_file = os.path.join(confpath, conffile)
        try: self.readfp(open(self.conf_file), 'r')
        except IOError as Err:
            if Err.errno == 2: pass
            else: raise Err

    def set(self, section, option, value):
        if self.has_section(section):
            ConfigParser.ConfigParser.set(self, section, option, str(value))
        else:
            self.add_section(section)
            ConfigParser.ConfigParser.set(self, section, option, str(value))

    def get(self, section, option):
        try: return Eval(ConfigParser.ConfigParser.get(self, section, option))
        except ConfigParser.NoOptionError: return None

    def save(self):
        self.write(open(self.conf_file, 'w'))

    def __del__(self):
        self.save()

class LocalConfig(_ConfParse):
    def __init__(self, conffile, confpath = '/etc/local/cnf'):
        _ConfParse.__init__(self, confpath, conffile)

class SysConfig(_ConfParse):
    def __init__(self, conffile, confpath = '/etc/sys/cnf'):
        _ConfParse.__init__(self, confpath, conffile)
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

One problem I see is that returning None from your modified get() method conflicts with the normal case of a valueless option (from the bottom of the module docs for ConfigParser):

>>> import ConfigParser
>>> import io

>>> sample_config = """
... [mysqld]
... user = mysql
... pid-file = /var/run/mysqld/mysqld.pid
... skip-external-locking
... old_passwords = 1
... skip-bdb
... skip-innodb
... """
>>> config = ConfigParser.RawConfigParser(allow_no_value=True)
>>> config.readfp(io.BytesIO(sample_config))

>>> # Settings with values are treated as before:
>>> config.get("mysqld", "user")
'mysql'

>>> # Settings without values provide None:
>>> config.get("mysqld", "skip-bdb")

>>> # Settings which aren't specified still raise an error:
>>> config.get("mysqld", "does-not-exist")
Traceback (most recent call last):
  ...
ConfigParser.NoOptionError: No option 'does-not-exist' in section: 'mysqld'

Note the second-to-last example commented as "Settings without values provide None:" Of course this isn't an issue if you intend to exclude this sort of option. Other than that, I like the auto-section feature.

Though, I'd lean towards adding to the interface rather than masking and changing the behavior, so instead of replacing get/set, add safe_ versions:

def safe_set(self, section, option, value):
    if self.has_section(section):
        self.set(section, option, str(value))
    else:
        self.add_section(section)
        self.set(section, option, str(value))

def safe_get(self, section, option):
    if self.has_option(section, option):
        return self.get(section, option)
    else:
        return None

This would make it more flexible as code would still have access to ConfigParser's normal interface and the option of using the "safe" calls which don't throw exceptions.

\$\endgroup\$
1
  • \$\begingroup\$ I thought about creating a different 'safe' method, but since I'm not doing anything to mess with the format of the file; you could still use the standard ConfigParser on the same file. Is retaining the inherited class methods un-modified a 'best' or 'standard' practice? I just don't know what the conventions are for something like this- if there are any! Thanks for looking at this! \$\endgroup\$
    – tMC
    Commented Jun 3, 2011 at 20:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.