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

Create core config for data/config/cache directories. #843

Closed
adamcik opened this Issue Sep 7, 2014 · 18 comments

Comments

3 participants
@adamcik
Member

adamcik commented Sep 7, 2014

Instead of having each backend set these up we should have these setup in core, like we do for proxy settings, and then extensions should just use $dir/$ext_name/ without having to do anything more. In the case of extensions which already define their own settings for this we should change the default something which refers to say $DATA_DIR/ext_name or just kill the settings outright.

@adamcik adamcik added this to the v0.20 - Audio cleanup milestone Sep 7, 2014

@adamcik adamcik added the 1 - Ready label Feb 4, 2015

@jodal

This comment has been minimized.

Member

jodal commented Feb 7, 2015

I suggest adding something like the following, with these default values:

[dir]
cache = $XDG_CACHE_DIR/mopidy
config = $XDG_CONFIG_DIR/mopidy
data = $XDG_DATA_DIR/mopidy

Should we extend mopidy.utils.path.expand_path() to take config['dir'] as an argument and use it to replace e.g. $CACHE_DIR, $CONFIG_DIR, and $DATA_DIR in paths with the proper values from config['dir']? I suggest we keep the $XDG_* variable support, but promote our own vars for most uses as they point directly to the mopidy dir in each XDG dir.

Other relevant configs in Mopidy core:

  • local/data_dir
    • Either change default to $DATA_DIR/local, or
    • Remove in favor of os.path.join(config['dir']['data'], 'local')
  • local/media_dir -- Unaffected.
  • local/playlists_dir -- Change default to $DATA_DIR/local/playlists.

Comments welcome!

@adamcik

This comment has been minimized.

Member

adamcik commented Feb 10, 2015

Sounds reasonable, that is as long as we don't end up with a circular resolution.

@adamcik adamcik modified the milestones: v0.21 - Gapless playback, v0.20 - Audio cleanup 1 Mar 13, 2015

@adamcik

This comment has been minimized.

Member

adamcik commented May 8, 2015

Put this under a core schema perhaps?

@jodal

This comment has been minimized.

Member

jodal commented May 8, 2015

Like this?

[core]
cache_dir = $XDG_CACHE_DIR/mopidy
config_dir = $XDG_CONFIG_DIR/mopidy
data_dir = $XDG_DATA_DIR/mopidy
@adamcik

This comment has been minimized.

Member

adamcik commented May 8, 2015

Mhm, would also be where a max_tracklist_length setting would fit in eventually.

@adamcik

This comment has been minimized.

Member

adamcik commented May 9, 2015

As mentioned in mopidy/mopidy-local-sqlite#59 (comment) a reasonable way to expose these dirs to extensions would be to combine the handling of this and get_or_create_dir in a Extension.get_data_dir() class method. We just need to make sure that the initial config pass gets the core scheme in addition to logging.

@tkem

This comment has been minimized.

Member

tkem commented May 9, 2015

Would it make sense to also add similar get_config_dir and get_cache_dir methods? Though I currently don't see a clear distinction between data_dir and cache_dir for mopidy-local-sqlite...

@adamcik

This comment has been minimized.

Member

adamcik commented May 9, 2015

Yes I would add all three with clear documentation with what each should be used for. For mopidy-local-sqlite data dir is probably all you need. A cache dir would be for things like caching album art that can be refetched / generated as one possible usage example.

@tkem

This comment has been minimized.

Member

tkem commented May 9, 2015

Another question is how to handle hyphenated ext_names, like local-sqlite or podcast-itunes, e.g whether these should be converted to subdirectories.

@tkem

This comment has been minimized.

Member

tkem commented May 9, 2015

@adamcik: totally agree with using cache_dir for extracted album art, but in principle the sqlite database can also be recreated from local scan, so it's also just eime sort of "cached" information.
However, this should be up to individual extensions to figure out, and providing all three methods sounds good to me.
Another question: should we mak cache_dir and data_dir different directories by default, so extensions can for example wipe/remove cache_dir on refresh? Should we also provide extension method for "clearing" cache_dir and/or data_dir?

@adamcik

This comment has been minimized.

Member

adamcik commented May 9, 2015

http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html is what these map to BTW, as for how they are to be used. So cache_dir normally ends up being ~/.cache/mopidy/... and it should always be safe for me to just do rm -rf ~/.cache, so yes data_dir and cache_dir need to be distinct IMO.

@tkem

This comment has been minimized.

Member

tkem commented Jun 3, 2015

This would really be awesome to have in v1.1, since it perfectly fits some of the things I have in mind for podcasts.

@tkem

This comment has been minimized.

Member

tkem commented Jun 3, 2015

Q: Could the "config" dir contain files that are not parsed by Mopidy at startup? For example XML files in a domain-specific format?

@jodal

This comment has been minimized.

Member

jodal commented Jun 3, 2015

Yet, it can. Mopidy will only care about $config_dir/mopidy.conf and any file referred to in mopidy.conf, like e.g. logging.conf.

@tkem

This comment has been minimized.

Member

tkem commented Jun 3, 2015

@jodal: I was thinking about init scripts etc., which (AFAIK) used filesystem globs for generating a list of config files that was passed to Mopidy. Can't remember exactly, but use case would be an extension that gets/creates a "private" config dir and wants to put some XML file into that. So that shouldn't conflict with any config processing done by Mopidy core.

@jodal

This comment has been minimized.

Member

jodal commented Jun 3, 2015

That is only the Debian package, which includes config from /usr/share/mopidy/conf.d/. That dir is only intended to be used by the Debian packages of other Mopidy extensions, so they can install default config for run-as-system-service that differs from run-as-user.

The only use of that today is Mopidy-Spotify is use it setup a different cache dir when it's a system service. With this feature implemented, and Mopidy-Spotify using core/cache_dir instead, the Mopidy-Spotify Debian package will no longer need to put any default config in /usr/share/mopidy/conf.d/.

@tkem

This comment has been minimized.

Member

tkem commented Jun 3, 2015

Thanks @jodal, couldn't remember the details; so this shouldn't be an issue.

@adamcik adamcik modified the milestones: Europython 2015 sprint, v1.1 - Robust core Jun 24, 2015

dprokic pushed a commit to dprokic/mopidy that referenced this issue Jul 25, 2015

dprokic pushed a commit to dprokic/mopidy that referenced this issue Jul 25, 2015

Dejan Prokić
config: Reorder core config options
As suggested by jodal on pull request mopidy#1232
Issue mopidy#843

@jodal jodal removed the 1 - Ready label Jul 26, 2015

@jodal

This comment has been minimized.

Member

jodal commented Jul 26, 2015

Fixed by #1232.

@jodal jodal closed this Jul 26, 2015

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