| Message ID | 86zg8k8iy8.fsf@conses.eu |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#62102] services: Add whoogle-service-type. | expand |
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
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)))) >> >> + > > 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
On 2023-06-09 23:03, Ludovic Courtès wrote: > Hi conses, > > Did you have a chance to look into Bruno’s suggestions below? > > Please let us know what you think. > Hey Ludovic, Yes, I've noted them but I haven't gotten around to implementing them yet. I'll give this a shot soon. > > 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)))) >>> >>> + >> >> 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
Hey, Miguel Ángel Moreno <contact@conses.eu> skribis: > On 2023-06-09 23:03, Ludovic Courtès wrote: > >> Hi conses, >> >> Did you have a chance to look into Bruno’s suggestions below? >> >> Please let us know what you think. >> > > Hey Ludovic, > > Yes, I've noted them but I haven't gotten around to implementing them > yet. I'll give this a shot soon. Friendly ping! :-) Ludo’.
On 2023-08-08 17:26, Ludovic Courtès wrote: > Hey, > Hi Ludovic, > Miguel Ángel Moreno <contact@conses.eu> skribis: > >> On 2023-06-09 23:03, Ludovic Courtès wrote: >> >>> Hi conses, >>> >>> Did you have a chance to look into Bruno’s suggestions below? >>> >>> Please let us know what you think. >>> I addressed Bruno's points, added extra fields to configure this service further, and documented these in the manual. In regard to tests, I had a glance at ./tests/services/ to see what kind of tests there were, and most them test serialization or some service-specific feature. Since Whoogle is essentially a Flask web application, I wouldn't know how to go about testing it in the context of a service. Other than that, happy to hear your feedback on the new revision :) >> >> Hey Ludovic, >> >> Yes, I've noted them but I haven't gotten around to implementing them >> yet. I'll give this a shot soon. > > Friendly ping! :-) > > Ludo’.
Hi, Friendly ping on this patch :) -- Best regards, Miguel Ángel Moreno
diff --git a/gnu/services/web.scm b/gnu/services/web.scm index d56e893527..66cc640a6d 100644 --- a/gnu/services/web.scm +++ b/gnu/services/web.scm @@ -35,6 +35,7 @@ (define-module (gnu services web) #:use-module (gnu services) #:use-module (gnu services shepherd) #:use-module (gnu services admin) + #:use-module (gnu services configuration) #:use-module (gnu services getmail) #:use-module (gnu services mail) #:use-module (gnu system pam) @@ -46,6 +47,7 @@ (define-module (gnu services web) #:use-module (gnu packages patchutils) #:use-module (gnu packages php) #:use-module (gnu packages python) + #:use-module (gnu packages python-web) #:use-module (gnu packages gnupg) #:use-module (gnu packages guile) #:use-module (gnu packages logging) @@ -235,6 +237,9 @@ (define-module (gnu services web) varnish-service-type + 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)))) + +;;; +;;; Whoogle +;;; + +(define-configuration/no-serialization whoogle-configuration + (whoogle + (package whoogle-search) + "The @code{whoogle-search} package to use.")) + +(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")) + #:environment-variables + (append (list "CONFIG_VOLUME=/var/cache/whoogle-search") + (default-environment-variables)))) + (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))) + (default-value (whoogle-configuration)) + (description "Run the @code{whoogle-search} engine."))) + ;;; ;;; Patchwork