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

Previous Next

Full log

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

From: Bruno Victal <mirai <at> makinata.eu>
To: conses <contact <at> conses.eu>
Cc: 62102 <at> debbugs.gnu.org, Andrew Tropin <andrew <at> trop.in>
Subject: Re: [bug#62102] [PATCH] services: Add whoogle-service-type.
Date: Fri, 31 Mar 2023 12:30:10 +0100

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.