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

Already on GitHub? Sign in to your account

New config format #280

Closed
jodal opened this Issue Dec 16, 2012 · 26 comments

Comments

Projects
None yet
3 participants
Owner

jodal commented Dec 16, 2012

We should consider to:

  • switch to ini files for configuration
  • support a cascading set of configuratiion files, e.g. default settings, /etc/mopidy.conf, /home/alice/.config/mopidy/mopidy.conf (would fix #267)
  • support loading additional config files using e.g. --config foo.conf. This could replace the current "profile" hack used by Mopidy developers to quickly switch between various configurations.

This would lead to easier configuration for the end user. Instead of the current situation where the config file must be a valid Python file, the strings must be unicode literals, and the settings names can get long, you could have something like this:

[spotify]
username = alice
password = secret

When activating an additional frontend, you currently need to:

  1. redefine the FRONTENDS setting,
  2. find and add it's three existing default values,
  3. add the fourth value.
  4. In addition, you'll miss out when we later add another frontend that is active by default.

Instead, you could activate another frontend by simply:

[http]
active = true

Since it's a lot easier to write ini files than Python files, you could also add interactive settings management, or a command line interface for getting and setting settings, like git config. E.g.:

$ mopidy config http.active
false
$ mopidy config http.active true

If we switch to a new configuration format, we must consider if it's worth the effort to automate conversion from the old Python settings files to the new format.

This was referenced Dec 29, 2012

Owner

adamcik commented Jan 3, 2013

We should probably have a run through of our current settings and see which ones can be moved over to such a new scheme with out a horrible mismatch between old and new. I for instance suspect that things that are lists today are poorly suited to be in such a simple INI format. This will need to be overcome in some sane way.

Other important questions / requirements that come to mind:

  • Should we just use stdlib or a new external dep.
  • Do we need to be able to modify config programmaticly, and if so should whitespace, comments etc be preserved?
  • How do we handle the section namespacing, if at all, and where do custom settings fit in (just a [custom] section)?
  • How do we preserve the current ability we have to handle default, local and runtime settings with respect to testing etc.

Also as mentioned in #279 something along the lines of the following for determining what to load might be an alternative to active = true:

[plugins]
mopidy.frontend.mpd = enabled
mopidy.backend.spotify = enabled
...

An other idea that comes to mind would be to us a [plugins] section to tell us what the sections for other modules are:

[plugins]
http = mopidy.frontend.http
spotify = mopidy.backend.spotify

[http]
port = 8080

...

But this is most likely a bad idea as it would be a pain in the ass to help users not knowing which section names the might have chosen and just asking for trouble.

Owner

adamcik commented Feb 26, 2013

@jodal mentioned https://github.com/mojombo/toml as a possible option, other one I've looked at before is http://www.voidspace.org.uk/python/configobj.html

Additional things to consider for any of the variants:

  • Is there some built in settings validation support that we should integrate against / port to?
  • If the format doesn't support loading multiple files, does it map to a simple enough datastructure that we can handle this ourselves?

@adamcik adamcik referenced this issue Mar 16, 2013

Closed

Add init script #266

Owner

adamcik commented Mar 17, 2013

There is also the new python3 configparser which is a somewhat shinier version of ConfigParse, though if we stick with the oldschool ConfigParse we could be using configglue to easily allow for command line overrides of settings. Also an advantage of configobj that I don't think I made clear is that we could create schemas and have them verified. Though given that we already have our own verification code that we could port to configparse this may not be a huge win. https://github.com/dnephin/PyStaticConfiguration was also suggested as an alternative.

Currently I think I have decided that my vote will be for moving to ini-ish config. So the next step will be just trying to convert all our existing config to a sample ini irrespective of what we end up using.

The other slight feature creep issue introduced by the proposal in #343 is that plugins will need to be able to have their own config sections that they need to validate. This probably implies that we will be ignoring config in sections nothing knows about so that we don't break stuff when a config is not loaded.

Owner

adamcik commented Mar 17, 2013

I also suspect that we just using ConfigParse and something along the lines of -s section.key=value can get a lot of what we need / want to support.

As for integrating logging config, I would like that we simply support the same ini format that logging supports, but have it in [logging.handlers] instead of [handlers]. This implies that we will need to pull out the values as us the dictconfig api instead when passing it on.

Owner

adamcik commented Mar 17, 2013

Oh, and as for plugin enablement with #343 this will mainly just be gated on if you have it installed or not. So if it is on your system it is on by default. If we where to say that the plugins themselves choose their entry point name, we could then support disabling via a simple blacklist.

Owner

jodal commented Mar 18, 2013

I did the exercise of mapping our current settings to ini syntax.

Open questions:

  1. Should we prefix the config sections of extensions? E.g. [ext.spotify]. The only non-extension sections we have in the above example is logging and audio, though I see the point of not risking collisions between future sections added by us and extensions we don't know about.
  2. How will extensions provide their default config? The mopidy.ini we ship will necessarily only contain the non-extension sections of the configuration file (and maybe the default config of extensions shipped with core).
  3. Should we explicitly pass the options object to all extensions, instead of relying on today's "magic" mopidy.settings object? Would that remove the need for today's default/local/runtime setting dicts and clearing of module state between tests? Or should we keep config as module-state? The previously mentioned staticconf also goes the module-state way.
Owner

adamcik commented Mar 20, 2013

Other logging related issue to add is #245 as there should be some way to set name.of.logger = DEBUG where DEBUG is either a log level enum or an int. This is important to consider with respect to how a -s logging.levels:name.of.logger=DEBUG type syntax would work on the command line. I.e. what to use as separators and what chars to allow in section and key names.

Owner

jodal commented Apr 8, 2013

When we update the docs wrt to the new config format, we should:

  • Consider what is the most user friendly content of a default ~/.config/mopidy/mopidy.conf
  • Create a :config: Sphinx helper or similar for linking to config value docs
Owner

adamcik commented Apr 8, 2013

My current TODO items related to this issue

  • Create a mopidy-convert-config that reads old settings maps them to new ones and serializes via schemas.
  • Convert the scanner fully to new config
  • Get rid of mopidy.utils.config in favor of mopdy.config.{schemas,values,...}.
  • Figure out how to handle use cases where a missing value would benefit from a more tailored error message.
  • Config values need to be strict about kwargs they support.
  • Rename mopidy.utils.path.expand_path to just expand?
  • Read all config in binary mode, only decoding string and list config values and possibly support --config-encoding

@ghost ghost assigned adamcik Apr 8, 2013

HLFH commented Apr 10, 2013

So with Mopidy.conf and new config file format, what is the equivalent to this ?

FRONTENDS = (u'mopidy.frontends.mpd.MpdFrontend', u'mopidy.frontends.lastfm.LastfmFrontend')
MPD_SERVER_HOSTNAME = u'0.0.0.0'
MPD_SERVER_PASSWORD = u'mdppassword'
LOCAL_MUSIC_PATH = u'/home/nicolargo/Musiques'
SPOTIFY_USERNAME = u'spotifylogin'
SPOTIFY_PASSWORD = u'spotifypassword'
SPOTIFY_BITRATE = 320
LASTFM_USERNAME = u'lastfmlogin'
LASTFM_PASSWORD = u'lastfmpassword'

Owner

jodal commented Apr 10, 2013

We'll create a tool to convert old to new config before the 0.14 release, but currently, the above config maps to:

[mpd]
hostname = 0.0.0.0
password = mdppassword

[local]
media_dir = /home/nicolargo/Musiques

[spotify]
username = spotifylogin
password = spotifypassword
bitrate = 320

[scrobbler]
username = lastfmlogin
password = lastfmpassword

# These last sections two are just to get the same
# effect your FRONTENDS setting got

[http]
enabled = false

[mpris]
enabled = false
Owner

adamcik commented Apr 12, 2013

  • Add section names to schemas so we can just use a list of them instead of dicts (need to decide if we check extension schema name match or assign).
  • ConfigError should be list of ('section/name: msg', 'section: missing...') entries.
  • Rename logging.levels to loglevels?
  • Support external logging config file logging/config_file?
  • Common mopidy.config.read for reading defaults in extensions etc?
  • Read all files in binary mode.
Owner

jodal commented Apr 13, 2013

+1 for renaming logging.levels to loglevels.

Owner

adamcik commented Apr 13, 2013

  • Make sure logging is bootstraped early enough to catch config errors when developing
Owner

adamcik commented Apr 13, 2013

  • Handle u'media_dir': '/home/adamcik/dev/mopidy/None' type cases for paths.
Owner

jodal commented Apr 14, 2013

I've tweaked and published the config API as is: http://docs.mopidy.com/en/develop/api/config/

Please keep it up to date with further developments.

Owner

jodal commented Apr 14, 2013

  • Handle \n in config values (ref. logging/debug_format)

jodal added a commit that referenced this issue Apr 15, 2013

Owner

adamcik commented Apr 16, 2013

  • Add more encoding related tests?
Owner

adamcik commented Apr 17, 2013

  • XDG_DIRS should not be unicode
  • We should check for non bytes paths
  • We should not create media dir
  • We should warn about missing media dir
  • types.Path().serialize being called with unicode should use sys.getfilesystemencoding() instead of utf-8

I know some of this is getting out of scope for this bug, but it's just more convenient to add here.

Owner

adamcik commented Apr 17, 2013

  • Always print exceptions that bubble up and are not handled, needed in case where it happens before logging is setup.
  • Convert config from extensions that is unicode into bytes?
  • Remove extension schema now that we can just do enabled in super?
  • Convert LogLevelSchema to a Wildcard on with WildCard('loglevel', LogLevel()) type interface (would need better name)?
Owner

adamcik commented Apr 25, 2013

  • Catch and handle parsing errors in config files.
Owner

adamcik commented Apr 27, 2013

  • Remove /etc/mopidy/mopidy.conf from default files to load?
Owner

jodal commented Apr 27, 2013

Because init scripts can add it again themselves through --config?

Owner

adamcik commented Apr 27, 2013

Correct, that location makes sense for the system installed mopidy service, end users should be loading $XDG_CONFIG_DIRS after having re-read the XDG spec. On a side note the spec confirms that we could be creating locations we intend to write in the way we do today :-)

Owner

adamcik commented Apr 27, 2013

I.e. default should be something along the lines of:

['%s/mopidy/mopidy.config' % d for d in glib.get_system_config_dirs() + (glib.get_user_config_dir(),)]
['/etc/xdg/mopidy/mopidy.config', '/home/adamcik/.config/mopidy/mopidy.config']
Owner

adamcik commented Apr 27, 2013

Opening a new issue for XDG stuff, only other open thread is converting unicode in the PathConfig val, so closing this now.

@adamcik adamcik closed this Apr 27, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment