HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python configuration module for a web framework

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
moduleforpythonwebconfigurationframework

Problem

I'm currently writing a web framework and I wanted to add a configuration module for reading environment variables and a configuration toml file.

Here is the code. The comment in the Config class should explain it all.

``
import os
import sys
import toml

BASEPATH = os.path.dirname(sys.modules['__main__'].__file__)
print(BASEPATH)
FILEPATH = os.path.abspath(os.path.join(BASEPATH, 'config.toml'))
print(FILEPATH)

class ConfigError(KeyError):
""" Raised when a requested configuration can not be found """

class ConfigBase:
def __init__(self):
self.basename = ''
self.default_options = {}
raise TypeError('Should not use ConfigBase directly')

def __getitem__(self, item):
""" called when access like a dictionary
config['database']['username']
"""
default = self.default_options.get(item)
if isinstance(default, dict):
return ConfigSection(self.basename + '.' + item, default)
# Check for env var
envvar_string = '_'.join(self.basename.split('.') + [item]).upper()
env = os.getenv(envvar_string)
if env is None:
if default is None:
raise ConfigError(
'No configuration found for {}. '
'Add the environment variable {} or add'
' the key to your config.toml file'.format(
'.'.join((self.basename, item)),
envvar_string
))
return default
return env

class Config(ConfigBase):
""" load configuration from envvar or config file
You can get settings from environment variables or a config.toml file
Environment variables have a higher precedence over the config file

Use
config['section']['subsection']['key']` to get the value
SECTION_SUBSECTION_KEY from environment variables or
section.subsection.key from a toml file
(usually written:

Solution

toml is a third-party library, so should be separated from the standard library imports:

import os
import sys

import toml


ConfigBase.__init__ seems a little strange:

def __init__(self):
    self.basename = ''
    self.default_options = {}
    raise TypeError('Should not use ConfigBase directly')


Why bother to set the attributes if you're then going to throw an error? Also you should use NotImplementedError for abstract base classes, by convention.

Config.load_config seems to be internal; are you expecting it to be called from anywhere other than Config.__init__? If not, rename it _load_config to indicate that it's an implementation detail. Also note that the temporary assignment to config is unnecessary:

def load_config(self):
    with open(FILEPATH, 'r') as f:
        return toml.load(f)


I'd also note that it seems weird to add a docstring for __getitem__ that just describes what that method is for, but then not document the non-standard load_config at all. Your target audience should already know what __getattr__ is for (or where to find out).

Talking of __getattr__, there's an awful lot of complexity hiding in there. Every other method you've written is nice and short, but this could do with breaking down. To describe it:

  • Get the default value



  • If the default is a dictionary, wrap it up and return it immediately



  • If not, but there's an appropriate value in the environment, return that



  • If not, but the default exists, return the default



  • If not, throw an error



That's how I'd structure the method:

default = self._get_default_value(item)
if isinstance(default, dict):  # or the more general collections.abc.Mapping
    return self._create_config_section(item, default)  # or maybe return ConfigSection(self._create_basename(item), default)
env = self._get_value_from_env(item)
if env is not None:
    return env
if default is not None:
    return default
raise ConfigError(self._create_error_message(item))


This reads like the description I wrote above, operates at a consistent level of abstraction (not delving into the details of e.g. how to get a value from the environment or where the default comes from) and reduces the nesting.

You can split out other obvious helper methods, like self._create_env_var(item), along the way. Doing so might highlight to you that it's a little weird to do the same thing two different ways in the same method:

  • self.basename + '.' + item



  • '.'.join((self.basename, item))



Finally, I don't think the distinction between ConfigBase, Config and ConfigSection is really helping you. You interact with them the two child classes in much the same way; only the initialisation differs. Instead, I might be inclined to do:

class Config:

    def __init__(self, basename='', defaults=None):
        self.basename = basename  # should already be lstrip-ped
        if defaults is None:
            defaults = self._load_config()
        self.default_options = defaults

    def __getitem__(self, item):
        ...

    ...

Code Snippets

import os
import sys

import toml
def __init__(self):
    self.basename = ''
    self.default_options = {}
    raise TypeError('Should not use ConfigBase directly')
def load_config(self):
    with open(FILEPATH, 'r') as f:
        return toml.load(f)
default = self._get_default_value(item)
if isinstance(default, dict):  # or the more general collections.abc.Mapping
    return self._create_config_section(item, default)  # or maybe return ConfigSection(self._create_basename(item), default)
env = self._get_value_from_env(item)
if env is not None:
    return env
if default is not None:
    return default
raise ConfigError(self._create_error_message(item))
class Config:

    def __init__(self, basename='', defaults=None):
        self.basename = basename  # should already be lstrip-ped
        if defaults is None:
            defaults = self._load_config()
        self.default_options = defaults

    def __getitem__(self, item):
        ...

    ...

Context

StackExchange Code Review Q#152265, answer score: 3

Revisions (0)

No revisions yet.