Hi Bruno, Bruno Victal <mi...@makinata.eu> writes:
> On 2023-04-29 18:04, Maxim Cournoyer wrote: >> Hi Bruno, >> >> Bruno Victal <mi...@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
