diff mbox series

[bug#63402,v5,1/5] services: herd: Add a new 'current-service' procedure.

Message ID 4ae50adcd4cef9d26b26eb4456727538d61f064c.1684461197.git.maxim.cournoyer@gmail.com
State New
Headers show
Series Implement a dynamic IP monitoring feature. | expand

Commit Message

Maxim Cournoyer May 19, 2023, 1:59 a.m. UTC 
* gnu/services/herd.scm (current-service): New procedure, mostly reusing the
existing current-services.
(current-services): Implement in terms of the above procedure.
---
 gnu/services/herd.scm | 52 +++++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 19 deletions(-)

Comments

Ludovic Courtès May 22, 2023, 3 p.m. UTC | #1 
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> * gnu/services/herd.scm (current-service): New procedure, mostly reusing the
> existing current-services.
> (current-services): Implement in terms of the above procedure.

How about having (lookup-service name) that calls the ‘status’ action on
the given service and either returns a <live-service> or #f?

‘current-services’ might be implemented as (lookup-service 'root) but
this should be kept as an implementation detail.

Ludo’.
Maxim Cournoyer May 22, 2023, 11:22 p.m. UTC | #2 
Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> * gnu/services/herd.scm (current-service): New procedure, mostly reusing the
>> existing current-services.
>> (current-services): Implement in terms of the above procedure.
>
> How about having (lookup-service name) that calls the ‘status’ action on
> the given service and either returns a <live-service> or #f?

I'd rather keep the name 'current-service', because 'lookup-service' is
already a public procedure exported by Shepherd's (shepherd service)
module; it'd be confusing.

> ‘current-services’ might be implemented as (lookup-service 'root) but
> this should be kept as an implementation detail.

Yeah, that's my view on current-services being implemented in terms of
(current-service 'root).  It's a bit weird, but that's because the
underlying API is not symmetrical either.

Thanks for taking a look!
Ludovic Courtès May 24, 2023, 2:44 p.m. UTC | #3 
Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi,
>>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>
>>> * gnu/services/herd.scm (current-service): New procedure, mostly reusing the
>>> existing current-services.
>>> (current-services): Implement in terms of the above procedure.
>>
>> How about having (lookup-service name) that calls the ‘status’ action on
>> the given service and either returns a <live-service> or #f?
>
> I'd rather keep the name 'current-service',

There’s no notion of a “current service” in the Shepherd; that would be
confusing to me.

> because 'lookup-service' is already a public procedure exported by
> Shepherd's (shepherd service) module; it'd be confusing.

Yeah well, I think we should clarify the client/server architecture and
the context in which (shepherd …) modules are meant to be used.  I made
a first attempt:

  https://git.savannah.gnu.org/cgit/shepherd.git/commit/?id=d3d437a34bcb11fc416bf141181d8908064aeceb

However, what matters most to me is that the procedure names really
represent what they do.  With that in mind, it’s no surprise that the
procedure to look up a service is called ‘lookup-service’ in both
contexts.

Thanks,
Ludo’.
Maxim Cournoyer July 21, 2023, 2:15 a.m. UTC | #4 
Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Hi,
>>>
>>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>>
>>>> * gnu/services/herd.scm (current-service): New procedure, mostly reusing the
>>>> existing current-services.
>>>> (current-services): Implement in terms of the above procedure.
>>>
>>> How about having (lookup-service name) that calls the ‘status’ action on
>>> the given service and either returns a <live-service> or #f?
>>
>> I'd rather keep the name 'current-service',
>
> There’s no notion of a “current service” in the Shepherd; that would be
> confusing to me.

We already have current-services in the same module, documented as:

  Return the list of currently defined Shepherd services, represented as
  <live-service> objects.

It's already a public interface.  Here I was interested in having a
convenient way to retrieve a single live-service instead of the full
list, thus the singular version.

Does that make sense?
diff mbox series

Patch

diff --git a/gnu/services/herd.scm b/gnu/services/herd.scm
index 48594015fc..02c2fec20f 100644
--- a/gnu/services/herd.scm
+++ b/gnu/services/herd.scm
@@ -1,6 +1,7 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2016-2019, 2022-2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2017, 2020 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -51,6 +52,7 @@  (define-module (gnu services herd)
             live-service-canonical-name
 
             with-shepherd-action
+            current-service
             current-services
             unload-services
             unload-service
@@ -208,31 +210,43 @@  (define (live-service-canonical-name service)
   "Return the 'canonical name' of SERVICE."
   (first (live-service-provision service)))
 
-(define (current-services)
-  "Return the list of currently defined Shepherd services, represented as
-<live-service> objects.  Return #f if the list of services could not be
-obtained."
-  (with-shepherd-action 'root ('status) results
-    ;; We get a list of results, one for each service with the name 'root'.
+(define (current-service name)
+  "Return the currently defined Shepherd service NAME, as a <live-service>
+object.  Return #f if the service could not be obtained.  As a special case,
+@code{(current-service 'root)} returns all the current services."
+  (define (process-services services)
+    (resolve-transients
+     (map (lambda (service)
+            (alist-let* service (provides requires running transient?)
+              ;; The Shepherd 0.9.0 would not provide 'transient?' in
+              ;; its status sexp.  Thus, when it's missing, query it
+              ;; via an "eval" request.
+              (live-service provides requires
+                            (if (sloppy-assq 'transient? service)
+                                transient?
+                                (and running *unspecified*))
+                            running)))
+          services)))
+
+  (with-shepherd-action name ('status) results
+    ;; We get a list of results, one for each service with the name NAME.
     ;; In practice there's only one such service though.
     (match results
       ((services _ ...)
        (match services
          ((('service ('version 0 _ ...) _ ...) ...)
-          (resolve-transients
-           (map (lambda (service)
-                  (alist-let* service (provides requires running transient?)
-                    ;; The Shepherd 0.9.0 would not provide 'transient?' in its
-                    ;; status sexp.  Thus, when it's missing, query it via an
-                    ;; "eval" request.
-                    (live-service provides requires
-                                  (if (sloppy-assq 'transient? service)
-                                      transient?
-                                      (and running *unspecified*))
-                                  running)))
-                services)))
+          ;; Summary of all services (when NAME is 'root or 'shepherd).
+          (process-services services))
+         (('service ('version 0 _ ...) _ ...) ;single service
+          (first (process-services (list services))))
          (x
-          #f))))))
+          #f))))))                ;singleton
+
+(define (current-services)
+  "Return the list of currently defined Shepherd services, represented as
+<live-service> objects.  Return #f if the list of services could not be
+obtained."
+  (current-service 'root))
 
 (define (resolve-transients services)
   "Resolve the subset of SERVICES whose 'transient?' field is undefined.  This