Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loosen config path assumptions #75

Open
beasteers opened this issue Apr 20, 2020 · 6 comments
Open

Loosen config path assumptions #75

beasteers opened this issue Apr 20, 2020 · 6 comments

Comments

@beasteers
Copy link
Contributor

confuse tries to make some smart guesses about where config files are stored on the system, but it can be somewhat confusing and sometimes I would rather just hardcode specific locations that I can direct people to with certainty.

Basically, I'd like to be able to just do this:

config = confuse.LazyConfig('mypkg', paths='~/.mypkg/config.yml')

I apologize for just pasting a bunch of code, but I figured I'd illustrate the change. I was planning on submitting a pull request, but I figured I would open an issue first to discuss.

Basically, I decoupled the auto-config detection from the the class definition and now they live in 2 utility functions. Now you have the option to override the default config paths and the Configuration object is simpler and less dependent on the config location assumptions.

class Configuration(RootView):
    def __init__(self, appname, modname=None, paths=None, defaults=None, read=True):
        super(Configuration, self).__init__([])
        self.appname = appname
        self.modname = modname
        self._env_var = '{}DIR'.format(self.appname.upper())
        self._package_path = _package_path(modname) if modname else None

        # set/find config files - this preserves the original behavior by default
        self.paths = _as_list(paths) or self.user_config_files()[:1]
        self.defaults_paths = _as_list(defaults) or self.pkg_config_files()

        if read:
            self.read()

    def user_config_files(self):
        return find_user_config_files(self.appname, self._env_var)

    def pkg_config_files(self):
        return find_pkg_config_files(self._package_path)

    def read(self, user=True, defaults=True):
        if user:
            for filename in self.paths:
                self.set_file(filename)
        if defaults:
            for filename in self.defaults_paths:
                self.set_file(filename, default=True)


    def set_file(self, filename, default=False, ignore_missing=False):
        filename = os.path.abspath(filename)
        if os.path.isdir(filename): # provided directory, find file
            fname = DEFAULT_FILENAME if default else CONFIG_FILENAME
            filename = os.path.join(filename, fname)

        if ignore_missing and not os.path.isfile(filename):
            return
        self.set(ConfigSource(load_yaml(filename), filename, default=default))

    def dump(self):
        ...


def find_user_config_files(appname, env_var, config_fname=CONFIG_FILENAME):
    # If environment variable is set, use it.
    if env_var in os.environ:
        appdir = os.path.abspath(os.path.expanduser(os.environ[env_var]))
        if os.path.isfile(appdir): # allow user to set config explicitly
            cfgfiles = [appdir]
        else:
            cfgfiles = [os.path.join(appdir, config_fname)]
    else:
        # Search platform-specific locations. If no config file is
        # found, fall back to the first directory in the list.
        cfgfiles = [os.path.join(d, appname, config_fname) for d in config_dirs()]
        cfgfiles = [f for f in cfgfiles if os.path.isfile(f)] or cfgfiles[:1]

    # Ensure that the directory exists.
    for f in cfgfiles:
        os.makedirs(os.path.dirname(f), exist_ok=True)
    return cfgfiles

def find_pkg_config_files(package_path, config_fname=DEFAULT_FILENAME):
    return [os.path.join(package_path, config_fname)] if package_path else []

def _as_list(x):
    return (
        x if isinstance(x, list) else 
        list(x) if isinstance(x, tuple) else
        [x] if x else [])
@sampsyo
Copy link
Member

sampsyo commented Apr 21, 2020

Awesome, yes, this seems great to have, especially if you have a nonstandard place where you want to keep your configuration file. Please do open a PR if you get the chance.

Here are a few random ideas for the design:

  • Instead of specifying a path to a configuration file, what do you think about specifying a path to the directory containing the config file? Namely, you'd pass path='~/.mypkg' or similar. The reasons I suggest this are:
    • Confuse likes the idea of having a "configuration directory," relative to which paths in the configuration are resolved, for example. So it seems slightly more direct to specify this than the file it contains. We could consider a separate option to change the filename, perhaps…
    • It might be a bit simpler to implement because the existing user_config_path mechanism works by looking for the right directory. When a manual path is specified, we could just make the config_dir method return that path (possibly after checking the environment variable, if we still want that available as an override? Maybe not though).
  • I think it's probably fine to make this just a single path instead of a "cascading" list, unless it seems useful for your use case to do specify multiple paths. There's always a fallback: it's possible to .add() additional sources to your heart's content.
  • Looks like you'd also like to override the mechanism for searching for defaults inside the package with a hard-coded filename. But there's nothing magical about the default config itself; it just ends up being one more configuration source. So if we do allow multiple paths, then you can get the same effect by passing paths=[user_config, default_config] right there (and not specifying modname, which disables the ordinary defaults thing).

@beasteers
Copy link
Contributor Author

Instead of specifying a path to a configuration file, what do you think about specifying a path to the directory containing the config file?

This actually will take either a filename or a directory name. If you look in set_file, I added a check that looks if it's a directory and if it is, it will look for CONFIG_FILENAME or DEFAULT_FILENAME based on the value of default passed.
So passing '~/mypkg' and '~/mypkg/config.yaml' are actually equivalent, but it also allows you to do '~/mypkg/settings.yaml' (which is useful when migrating from legacy naming decisions.)

Confuse likes the idea of having a "configuration directory," relative to which paths in the configuration are resolved

Ah okay now I understand what you mean by config directory - yeah we can definitely alter this to continue to provide a config_dir interface - although we could probably change from a function to an attribute set in __init__.

I think it's probably fine to make this just a single path instead of a "cascading" list, unless it seems useful for your use case to do specify multiple paths.

Yeah I'm fine with that. I'm migrating a project from configparser which reads from a list which is why I thought of doing that. This takes either a single path or a list

Looks like you'd also like to override the mechanism for searching for defaults inside the package with a hard-coded filename.

Ok yeah the only reason I made it different was because the config file names are different and there's that default=True argument that goes to ConfigSource which it looks like is only relevant when dumping (and pulling comments from).

I see there's an issue about pulling from environment variables which is an idea I really like. It would feel very natural to be able to do something like this (strings assumed to be yaml files). I just get the priority between add and set mixed up so this is a very definitive way of setting your config order.

config = confuse.LazyConfig('mypkg', [
    confuse.Environment('PREFIX'),
    '~/mypkg',
    os.path.dirname(__file__),
])

I'll submit a pull request for this when I get a few minutes!

@sampsyo
Copy link
Member

sampsyo commented Apr 22, 2020

This all sounds great! Thanks for the extra elaboration. I like the idea of just being able to pass a list, which would be equivalent to getting the add and set imperative operations right after the fact.

@curtiscook
Copy link

Jumping in to say this is great!

Right now I'm doing this as a workaround so am looking forward to your 2 pr's going in

def load_environment_config() -> LazyConfig:
    config = LazyConfig("Service", __name__)
    config_file = os.path.join(
        ROOT_DIR, "../../config", "config-{env}.yaml".format(env=ENV)
    )
    config.set_file(config_file)
    return config

@ryanrichholt
Copy link
Contributor

ryanrichholt commented Jul 27, 2021

I think this will solve a problem that I have now - I would like to split the config into two files. Here's what I'm imagining:

config = confuse.Configuration('foo', __name__, filename='config.yaml')
other_config = confuse.Configuration('foo', __name__, filename='other-config.yaml')

I was considering adding the filename parameter to the Configuration init method, and it would use that or the CONFIG_FILENAME value.

@bendikro
Copy link

So did this ever get anywhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants