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

Previous Next

Full log

Message #11 received at 62102 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: conses <contact <at> conses.eu>
Cc: Bruno Victal <mirai <at> makinata.eu>, 62102 <at> debbugs.gnu.org,
 Andrew Tropin <andrew <at> trop.in>
Subject: Re: bug#62102: [PATCH] services: Add whoogle-service-type.
Date: Fri, 09 Jun 2023 23:03:51 +0200

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 <at> 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))))
>>  
>> +
>
> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.