bug#63082: [PATCH 14/17] services: mpd: Obsolete 'environment-variables' field.

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hi Bruno,

Bruno Victal <mirai@makinata.eu> writes:

> On 2023-04-29 18:04, Maxim Cournoyer wrote:
>> Hi Bruno,
>> 
>> Bruno Victal <mirai@makinata.eu> writes:
>> 
>>> On 2023-04-28 15:27, Maxim Cournoyer wrote:
>>>> Rationale: Services can be extended via the simple-service mechanism instead
>>>> of having to expose fields on service configurations that are not directly
>>>> connected to the service's configuration.
>>>>
>>>> * gnu/services/audio.scm (mpd-environment-variables-sanitizer): New sanitizer.
>>>> (mpd-configuration): Use it.
>>>> (mpd-shepherd-service): Hard code the useful environment variables inside the
>>>> Shepherd service.
>>>> ---
>>>
>>> This field shouldn't be deprecated as one of it's primary purposes is to allow for
>>> the pulseaudio daemon configuration to be set to another one.
>>> What you're doing here is effectively hardcoding the pulseaudio configuration.
>> 
>> Our only means to declare a pulseaudio configuration
>> (pulseaudio-service-type) places it at this location, so it seems
>> reasonable to hard code it.  What use case do you have for a custom
>> pulseaudio configuration that pulseaudio-service-type could not cater
>> to?  This prevents users defining another environment variable and
>> forgetting to replace these, then wondering why the default pulse
>> configuration doesn't work, and it felt out of place to me (an
>> implementation detail better encapsulated).
>
> Indeed but note that there's a small subtlety to pulseaudio-service-type, chiefly that
> the service is not your typical ¿monodaemonic? process that is used throughout the system,
> rather it simply provides you a default config for the pulseaudio daemon.
> The fact that multiple pulseaudio daemons can be launched alongside is a strong indicator that
> perhaps you will want for some of them to use different configurations, which is done via the
> environment variables.
>
> Right now this would be mainly achieved using local-file, text-file or specifying a path
> but in theory the procedures for pulseaudio-service-type could be reused for serializing
> configurations to be used outside of the service.
>
> Regarding the users forgetting the variables, it looks obvious that if you omit the default
> values there then the behavior will also change accordingly.
> If you strongly feel that this is very pitfall prone (IMO it's no worse than forgetting to add %base-services
> at the end of the services field) then perhaps documenting it would suffice?

Is this a use case in actual use?  It seems a bit of a stretch in my
mind, especially considering the service was already difficult to get
working in its default configuration; I doubt someone would go out of
their way to manage multiple distinctly configured pulseaudio daemons
:-).  But if it's something in actual use providing value, I don't mind
to keep it until we have a better way to extend a common set of basic
properties for services in general.

>>> I'd consider this field to be within the same category as
>>> 'shepherd-requirement', it's for flexibility
>> 
>> I like the idea of more flexibility, but I don't like that these fields
>> need to be duplicated for each service, somewhat encumbering the view.
>> Perhaps we need to devise some 'always nice to have' set that would be
>> configurable for any service without having to expose these fields as
>> part of their main configuration?
>> 
>
> Right, it's not optimal but these are fields with legitimate uses, instituting rigidity here will simply
> make the services overly opinionated to some particular kind of setup, which drastically reduces their value.
>
> Regarding their duplication, perhaps an improvement for define-record-type* would be better?
> SRFI-136 seems something that would address these concerns.

I've yet to look carefully into SRFI-136 would provide us with, but it
seems an interesting direction, to the sound of it.

-- 
Thanks,
Maxim