[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#54986] [PATCH 0/2] services: mpd: Refactor MPD service
From: |
Liliana Marie Prikler |
Subject: |
[bug#54986] [PATCH 0/2] services: mpd: Refactor MPD service |
Date: |
Thu, 08 Dec 2022 14:35:55 +0100 |
User-agent: |
Evolution 3.46.0 |
Am Donnerstag, dem 08.12.2022 um 13:11 +0000 schrieb mirai:
> On 2022-12-07 18:27, Liliana Marie Prikler wrote:
> > Note that there is [1], which attempts to make it so that shepherd
> > endpoints can be specified in lieu of MPD endpoints. You don't
> > need to implement this logic – you are free to do so if you want to
> > – but could you make it so that there is an explicit endpoint
> > abstraction that would allow for extension later on?
> >
> > Cheers
> >
> > [1] https://issues.guix.gnu.org/54986
>
> Hi,
>
> After reading issue #54986, regarding mpd escaping shepherd
> management, my guess is that there could be two issues at play here:
>
> * mpd.conf was serialized with pid_file [1]. The safest is
> to actually not serialize any unused directives and let MPD decide
> based on what's actually present. pid_file should only be set if
> MPD is to be launched as a "daemon process". [2]
>
> * But the service definition for MPD is launched with '--no-daemon'
> option as seen in [3], which could be triggering a bug in MPD. It's
> probably worth testing the combinations of having pid_file set and
> invoking --no-daemon.
Nice catch, but completely unrelated to the issue. In neither case do
we ever spawn MPD as a regular daemon, yet only in the systemd case
does shepherd have a problem with it. That statement was concerning
shepherd's (mis)management of sockets, not MPD.
> > but could you make it so that there is an explicit endpoint
> > abstraction that would allow for extension later on?
>
> If I'm understanding correctly what [1] is about, I think this can be
> implemented through the 'extend' interface. See how certbot adds a
> nginx-server-configuration to a nginx-service-type through the
> 'extend' interface. For MPD, this would amount to appending the
> 'adresses' field with "adequately formatted" values.
This doesn't work for #54986, which makes it so that in-file addresses
are ignored in favour of handing over the sockets directly through
shepherd. Looking at [4], it appears the meaning of "port" is closer
to that of a default port, as addresses can have ports in them. But I
would still prefer addresses to be "endpoints", which if they happen to
be a list of strings are taken as MPD addresses and if they happen to
be shepherd endpoints are passed on to the shepherd service.
WDYT?
[4] https://mpd.readthedocs.io/en/stable/user.html#client-connections
[bug#59866] [PATCH v2] services: mpd: Refactor MPD service., mirai, 2022/12/07
[bug#59866] [PATCH v3] services: mpd: Refactor MPD service., mirai, 2022/12/16
[bug#59866] [PATCH v4 1/2] services: mpd: use 'define-configuration'., mirai, 2022/12/21
[bug#59866] [PATCH v5 1/2] services: mpd: rewrite using 'define-configuration'., mirai, 2022/12/24
[bug#59866] [PATCH v5.1] services: mpd: Refactor MPD service., mirai, 2022/12/24