[shepherd] Stopping service twice should not say “not found”

[shepherd] Stopping service twice should not say “not found”

[Ricardo Wurmus]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

Hi,

requesting that a shepherd service (e.g. ’foo’) that has already been
stopped be stopped again produces the error "service 'foo' could not be
found".  This is confusing because it implies that ’foo’ is not a valid
name.

(shepherd service) defines a method “stop” that uses “lookup-running” to
find running services:

--8<---------------cut here---------------start------------->8---
;; Stopping by name.
(define-method (stop (obj <symbol>) . args)
  (let ((which (lookup-running obj)))
    (if (not which)
	(let ((unknown (lookup-running 'unknown)))
	  (if (and unknown
		   (defines-action? unknown 'stop))
	      (apply action unknown 'stop obj args)
              (raise (condition (&missing-service-error (name obj))))))
        (apply stop which args))))
--8<---------------cut here---------------end--------------->8---

I think it would be better to ignore “stop” when it’s run on a stopped
service.  Here’s a patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-service-Don-t-raise-error-when-stopping-an-already-s.patch --]
[-- Type: text/x-patch, Size: 1881 bytes --]

From c285d0f085e8e9c6a4ad347643f7186807735189 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Sun, 3 Feb 2019 23:25:22 +0100
Subject: [PATCH] service: Don't raise error when stopping an already stopped
 service.

* modules/shepherd/service.scm (stop): If a service by the given name exists
and is already stopped, then don't raise an error.
---
 modules/shepherd/service.scm | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index 34a55a1..9f38886 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -4,6 +4,7 @@
 ;; Copyright (C) 2014 Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
 ;; Copyright (C) 2016 Alex Kost <alezost@gmail.com>
 ;; Copyright (C) 2018 Carlo Zancanaro <carlo@zancanaro.id.au>
+;; Copyright (C) 2019 Ricardo Wurmus <rekado@elephly.net>
 ;;
 ;; This file is part of the GNU Shepherd.
 ;;
@@ -592,13 +593,17 @@ Used by `start' and `enforce'."
 ;; Stopping by name.
 (define-method (stop (obj <symbol>) . args)
   (let ((which (lookup-running obj)))
-    (if (not which)
-	(let ((unknown (lookup-running 'unknown)))
+    (if which
+	(apply stop which args)
+        (let ((unknown (lookup-running 'unknown)))
 	  (if (and unknown
 		   (defines-action? unknown 'stop))
 	      (apply action unknown 'stop obj args)
-              (raise (condition (&missing-service-error (name obj))))))
-        (apply stop which args))))
+              ;; Only print an error if the service does not exist.
+              (match (lookup-services name)
+                (()
+                 (raise (condition (&missing-service-error (name obj)))))
+                (_ #f)))))))
 
 (define-method (action (obj <symbol>) the-action . args)
   "Perform THE-ACTION on all the services named OBJ.  Return the list of
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 12 bytes --]


--
Ricardo

Re: [shepherd] Stopping service twice should not say “not found”

[Ludovic Courtès]

Hello,

Ricardo Wurmus <rekado@elephly.net> skribis:

> requesting that a shepherd service (e.g. ’foo’) that has already been
> stopped be stopped again produces the error "service 'foo' could not be
> found".  This is confusing because it implies that ’foo’ is not a valid
> name.

Indeed.

> From c285d0f085e8e9c6a4ad347643f7186807735189 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Sun, 3 Feb 2019 23:25:22 +0100
> Subject: [PATCH] service: Don't raise error when stopping an already stopped
>  service.
>
> * modules/shepherd/service.scm (stop): If a service by the given name exists
> and is already stopped, then don't raise an error.

LGTM, though could you add a test?  There are already tests that grep
the output of “herd stop foo”; you’d just grep it a second time and
ensure it’s empty, and also ensure that $? is zero.

Thanks!

Ludo’.