[bug#62102] [PATCH] services: Add whoogle-service-type.

["Ludovic Courtès" <ludo@gnu.org>]

Hi conses,

Did you have a chance to look into Bruno’s suggestions below?

Please let us know what you think.

Thanks in advance,
Ludo’.

Bruno Victal <mirai@makinata.eu> skribis:

> Hi,
>
> On 2023-03-10 20:11, conses wrote:
>>  
>> +            whoogle-service-type
>> +            whoogle-configuration
>> +
>
> [...]
>
>>              patchwork-database-configuration
>>              patchwork-database-configuration?
>>              patchwork-database-configuration-engine
>> @@ -1575,6 +1580,47 @@ (define varnish-service-type
>>     (default-value
>>       (varnish-configuration))))
>>  
>> +\f
>
> Missing page-break character here?
> If you're using Emacs you can insert one with C-q C-l.
>
>> +;;;
>> +;;; Whoogle
>> +;;;
>> +
>> +(define-configuration/no-serialization whoogle-configuration
>> +  (whoogle
>> +    (package whoogle-search)
>> +    "The @code{whoogle-search} package to use."))
>
> I prefer this field to be named 'package' instead, to make it less prone
> to confusion. The accessor, whoogle-configuration-package, should be exported
> as well.
>
>> +(define (whoogle-shepherd-service config)
>> +  (list
>> +   (shepherd-service
>> +    (provision '(whoogle-search))
>> +    (start #~(make-forkexec-constructor
>> +              (list (string-append #$(whoogle-configuration-whoogle config)
>> +                                   "/bin/whoogle-search"))
>
> In general, you may prefer to use match-record instead of using accessors
> as it results in much less code to type.
>
>> +              #:environment-variables
>> +              (append (list "CONFIG_VOLUME=/var/cache/whoogle-search")
>> +                      (default-environment-variables))))
>
> Is (default-environment-variables) necessary?
>
>> +    (stop #~(make-kill-destructor))
>> +    (documentation "Run a @code{whoogle-search} instance."))))
>> +
>> +(define (whoogle-profile-service config)
>> +  (list
>> +   (whoogle-configuration-whoogle config)))
>
> [...]
>
>> +
>> +(define whoogle-service-type
>> +  (service-type
>> +   (name 'whoogle-search)
>> +   (extensions
>> +    (list
>> +     (service-extension
>> +      shepherd-root-service-type
>> +      whoogle-shepherd-service)
>> +     (service-extension
>> +      profile-service-type
>> +      whoogle-profile-service)))
>
> You can use match-record here or use the accessor here instead.
> (e.g., (compose list whoogle-configuration-package))
>
>
> Could you add a system test for this service as well?
> It makes things easier to check and maintain.
>
>
> Cheers,
> Bruno