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

Running Mopidy as a daemon in Debian #590

Closed
wants to merge 3 commits into from
Closed

Running Mopidy as a daemon in Debian #590

wants to merge 3 commits into from

Conversation

tkem
Copy link
Member

@tkem tkem commented Nov 22, 2013

Hi y'all,

I forked the "debian" branch and added a small Debian package for running Mopidy as a system daemon.

The additional package is named "mopidy-daemon", depends on the "mopidy" package (so I could use the current version from the repo), and provides:

  • a system user "mopidy" in group "audio" with home /var/lib/mopidy
  • directories /var/cache/mopidy, /var/lib/mopidy/media, /var/lib/mopidy/playlists, /var/lib/mopidy/log
  • an init script /etc/init.d/mopidy-daemon
  • default configuration for init script in /etc/default/mopidy-daemon
  • minimum default configuration in /etc/mopidy/mopidy.conf, with setup for local music/playlists in /var/lib/mopidy/{media,playlists}
  • separate logging configuration in /etc/mopidy/logging.conf, with log output directed to /var/log/mopidy/mopidy.log
  • running "/etc/init.d/mopidy-daemon force-reload" invokes mopidy-scan
  • except for adding the new package to debian/control, no interference with existing code

I'd greatly appreciate if you gave this a look, and let me know what you think of it. I've tested on Ubuntu 13.10 and Raspbian. My primary goal for this was to replace MPD on my Raspberry Pi "kitchen radio"; looks good, so far ;-)

Kind Regards,
Thomas

@jodal
Copy link
Member

jodal commented Nov 22, 2013

Cool! I'll give this a look later :-)

@ghost ghost assigned jodal Nov 22, 2013
@adamcik
Copy link
Member

adamcik commented Nov 23, 2013

Great idea splitting out the daemon config from the main server like this :-)

@tkem
Copy link
Member Author

tkem commented Nov 23, 2013

Well, thanks ;-) The idea was to keep the whole daemon-thing self-contained, and keep the standard mopidy package as-is. A lot of people seem to have invested quite some effort to get mopidy running as a daemon locally, so they might not want to install the daemon package, since that would clash with their "homegrown" installation.

One area that needs some clarification, though, is the use of /etc/mopidy/mopidy.conf

My version of mopidy does not seem to use it when running locally, but mopidy-scan does, according to debug output. It would also be great if mopidy-scan had a --config option, so I could use that when running it from the init script on "force-reload".

@adamcik
Copy link
Member

adamcik commented Nov 23, 2013

mopidy-scan is being moved to a sub-command of mopidy, this means that the behavior is unified with respect to --config

@adamcik
Copy link
Member

adamcik commented Nov 23, 2013

Hmm, this idea is probably something that doesn't need to fit into this initial PR, but we probably want a /etc/mopidy/extensions.d/{spotify,soundcloud,...}.conf as we need to be able to let the other debian packages such as mopidy-spotify install config files as well. So this would basically end up setting --config to first all the .d files and finally /etc/mopidy/mopidy.conf

@tkem
Copy link
Member Author

tkem commented Nov 24, 2013

Having a separate directory for extensions is definitely a good idea; the way this is generally done is having a setting in your master config file, e.g.

config_dir = /etc/mopidy/extensions.d

or

config_files = /etc/mopidy/extensions.d/*.conf

so the files will usually be included after parsing your master config.

So this could be implemented later without any change to --config behavior.

@tkem
Copy link
Member Author

tkem commented Nov 24, 2013

Sorry, make this include_dir or include_files for clearer semantics.

@adamcik
Copy link
Member

adamcik commented Nov 25, 2013

I'll have to think a bit about how I want to solve this, was happy with just having gotten rid of loading the config in two phases. But not really a huge deal if we re-add it.

Alternative I'm considering is a flag for the same thing, and have it default to $XDG_CONFIG_DIRS and search for mopidy/*conf, or just have --config-include-dir=...

But I digress again, this is useful to figure out, but not relevant mergin for this PR. Hopefully @jodal will have some time soon to look at the packaging changes :-)

@@ -0,0 +1,22 @@
[loggers]
keys=root
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do s/=/ = /g for increased readability here as it's an ini file, not a shell script.

@jodal
Copy link
Member

jodal commented Nov 25, 2013

I think this looks really good!

I will want to include this in the mopidy package and not as a separate package. That's whats makes sense when getting Mopidy into Debian. All the people having various init script setups will probably be happy with a way more complete and maintained init script setup.

That said, please continue to keep the packages separated for now, and I'll merge them later, but name variables etc with the mopidy package in mind, e.g. drop "daemon" from most places.

@@ -0,0 +1,6 @@
#!/bin/sh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file can be completely dropped since it only contains the debhelper token.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this was necessary for a prerm script with a proper stop command to be generated, but this seems to be handled by dh_installinit anyway. Thanks for the hint.

@jodal
Copy link
Member

jodal commented Nov 25, 2013

@gitkemmer For some much needed context, @tfheen is the Debian Developer that will sponsor the Mopidy packages into Debian. Consider his words my words :-)

@tkem
Copy link
Member Author

tkem commented Nov 26, 2013

Yeah, I know: Always be nice to your Debian sponsor ;-)

@tkem
Copy link
Member Author

tkem commented Nov 26, 2013

@jodal, so it doesn't get lost in the commit comments: @tfheen suggested removing the .default file altogether, since there's no real reasing for overriding these settings for an end user/admin,

The .default was convenient to get this up and running, but since this stuff may now be included in the official mopidy package, I'd be happy to get rid of it.

Can we agree on this? We'd have to clarify the default options (currently --quiet), though.

@jodal
Copy link
Member

jodal commented Nov 26, 2013

I'm all good with removing the *.default file.

I think having --quiet in the default args makes lots of sense when we're running as a daemon. We want logging to file, not to stdout.

@tfheen
Copy link

tfheen commented Nov 26, 2013

With proper init systems (cough systemd cough), logging to stdout is fine, but you can have that difference if you start shipping a .service file. :-)

@tkem
Copy link
Member Author

tkem commented Nov 29, 2013

Hi all,
I think I now made most of the changes suggested by @jodal and @tfheen; stubbornly refused to rename $DAEMON_USER to $USER, though ;-)
I also rebased this on the current 0.17.0 debian branch and converted the init-script "force-reload" option to run mopidy local scan sub-command.
Please have another look.

@tkem
Copy link
Member Author

tkem commented Dec 2, 2013

@tfheen: for now, the "mopidy-daemon" package is kept separate, as is the init script; @jodal will merge this into the regular "mopidy" package at his convenience. This will also include renaming "mopidy-daemon.init" to "mopidy.init", I guess.

The "- documentation" was an oversight, of course. Thanks for reporting!

@tfheen
Copy link

tfheen commented Dec 3, 2013

Renaming init scripts is a pain, I'd recommend changing the init script name straight away even if you keep the init script in its own package.

@jodal
Copy link
Member

jodal commented Dec 4, 2013

I've merged (and squashed) this into the debian branch now, tested a bit, and done some tweaks.

Next up, Mopidy needs support combining multiple config files, so that the extension packages can install their own config fragments and use e.g. /var/cache/mopidy out of the box.

@jodal jodal closed this Dec 4, 2013
This was referenced Dec 4, 2013
@tkem tkem deleted the debian-daemon branch August 22, 2014 07:26
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

Successfully merging this pull request may close these issues.

None yet

4 participants