Use exceptions to signal failure in add and remove.

Rename drop to remove.

src/supervisor/rpcinterface.py

@@ -186,9 +186,11 @@ class SupervisorNamespaceRPCInterface:
 
         for config in self.supervisord.options.process_group_configs:
             if config.name == name:
-                return self.supervisord.add_process_group(config)
-
-        return False
+                result = self.supervisord.add_process_group(config)
+                if not result:
+                    raise RPCError(Faults.ALREADY_ADDED, name)
+                return True
+        raise RPCError(Faults.BAD_NAME, name)
 
     def removeProcess(self, name):
         """ Remove a stopped process from the active configuration.
@@ -198,9 +200,12 @@ class SupervisorNamespaceRPCInterface:
         """
         self._update('removeProcess')
         if name not in self.supervisord.process_groups:
-            return False
+            raise RPCError(Faults.BAD_NAME, name)
 
-        return self.supervisord.remove_process_group(name)
+        result = self.supervisord.remove_process_group(name)
+        if not result:
+            raise RPCError(Faults.STILL_RUNNING)
+        return True
 
     def _getAllProcesses(self, lexical=False):
         # if lexical is true, return processes sorted in lexical order,

src/supervisor/supervisorctl.py

@@ -677,28 +677,46 @@ class DefaultControllerPlugin(ControllerPluginBase):
 
         supervisor = self.ctl.get_supervisor()
         for name in names:
-            result = supervisor.addProcess(name)
-            if result:
-                self.ctl.output("%s: added process" % name)
+            try:
+                supervisor.addProcess(name)
+            except xmlrpclib.Fault, e:
+                if e.faultCode == xmlrpc.Faults.SHUTDOWN_STATE:
+                    self.ctl.output('ERROR: shutting down')
+                elif e.faultCode == xmlrpc.Faults.ALREADY_ADDED:
+                    self.ctl.output('ERROR: process group already active')
+                elif e.faultCode == xmlrpc.Faults.BAD_NAME:
+                    self.ctl.output(
+                        "ERROR: no such process/group: %s" % name)
+                else:
+                    raise
             else:
-                self.ctl.output("%s: ERROR (no such group or already active)" % name)
+                self.ctl.output("%s: added process group" % name)
 
     def help_add(self):
-        self.ctl.output("add <name> [...]\tActivates any updates in config for process/group")
+        self.ctl.output("add <name> [...]\tActivates any updates in config "
+                        "for process/group")
 
-    def do_drop(self, arg):
+    def do_remove(self, arg):
         names = arg.strip().split()
 
         supervisor = self.ctl.get_supervisor()
         for name in names:
-            result = supervisor.removeProcess(name)
-            if result:
-                self.ctl.output("%s: dropped" % name)
+            try:
+                result = supervisor.removeProcess(name)
+            except xmlrpclib.Fault, e:
+                if e.faultCode == xmlrpc.Faults.STILL_RUNNING:
+                    self.ctl.output('ERROR: process/group still running: %s'
+                                    % name)
+                elif e.faultCode == xmlrpc.Faults.BAD_NAME:
+                    self.ctl.output(
+                        "ERROR: no such process/group: %s" % name)
+                else:
+                    raise
             else:
-                self.ctl.output("%s: ERROR (no such group or already dropped)" % name)
+                self.ctl.output("%s: dropped" % name)
 
-    def help_drop(self):
-        self.ctl.output("drop <name> [...]\tRemoves process/group from active config")
+    def help_remove(self):
+        self.ctl.output("remove <name> [...]\tRemoves process/group from active config")
 
     def do_refresh(self, arg):
         pass

src/supervisor/tests/base.py

@@ -770,12 +770,24 @@ class DummySupervisorRPCNamespace:
         return [[['added'], ['changed'], ['dropped']]]
 
     def addProcess(self, name):
+        from xmlrpclib import Fault
+        from supervisor import xmlrpc
+        if name == 'ALREADY_ADDED':
+            raise Fault(xmlrpc.Faults.ALREADY_ADDED, '')
+        if name == 'BAD_NAME':
+            raise Fault(xmlrpc.Faults.BAD_NAME, '')
         if hasattr(self, 'processes'):
             self.processes.append(name)
         else:
             self.processes = [name]
 
     def removeProcess(self, name):
+        from xmlrpclib import Fault
+        from supervisor import xmlrpc
+        if name == 'STILL_RUNNING':
+            raise Fault(xmlrpc.Faults.STILL_RUNNING, '')
+        if name == 'BAD_NAME':
+            raise Fault(xmlrpc.Faults.BAD_NAME, '')
         self.processes.remove(name)
 
     def clearProcessStdoutLog(self, name):

src/supervisor/tests/test_rpcinterfaces.py

@@ -235,6 +235,7 @@ class SupervisorNamespaceXMLRPCInterfaceTests(TestBase):
 
     def test_addProcess(self):
         from supervisor.supervisord import Supervisor
+        from supervisor import xmlrpc
         options = DummyOptions()
         supervisord = Supervisor(options)
         pconfig = DummyPConfig(options, 'foo', __file__, autostart=False)
@@ -247,12 +248,12 @@ class SupervisorNamespaceXMLRPCInterfaceTests(TestBase):
         self.assertTrue(result)
         self.assertEqual(supervisord.process_groups.keys(), ['group1'])
 
-        result = interface.addProcess('group1')
-        self.assertTrue(not result)
+        self._assertRPCError(xmlrpc.Faults.ALREADY_ADDED,
+                             interface.addProcess, 'group1')
         self.assertEqual(supervisord.process_groups.keys(), ['group1'])
 
-        result = interface.addProcess('asdf')
-        self.assertTrue(not result)
+        self._assertRPCError(xmlrpc.Faults.BAD_NAME,
+                             interface.addProcess, 'asdf')
         self.assertEqual(supervisord.process_groups.keys(), ['group1'])
 
     def test_removeProcess(self):
@@ -270,6 +271,36 @@ class SupervisorNamespaceXMLRPCInterfaceTests(TestBase):
         self.assertTrue(result)
         self.assertEqual(supervisord.process_groups.keys(), [])
 
+    def test_removeProcess_bad_name(self):
+        from supervisor.supervisord import Supervisor
+        from supervisor import xmlrpc
+        options = DummyOptions()
+        supervisord = Supervisor(options)
+        pconfig = DummyPConfig(options, 'foo', __file__, autostart=False)
+        gconfig = DummyPGroupConfig(options, 'group1', pconfigs=[pconfig])
+        supervisord.options.process_group_configs = [gconfig]
+
+        interface = self._makeOne(supervisord)
+
+        self._assertRPCError(xmlrpc.Faults.BAD_NAME,
+                             interface.removeProcess, 'asdf')
+
+    def test_removeProcess_still_running(self):
+        from supervisor.supervisord import Supervisor
+        from supervisor import xmlrpc
+        options = DummyOptions()
+        supervisord = Supervisor(options)
+        pconfig = DummyPConfig(options, 'foo', __file__, autostart=False)
+        gconfig = DummyPGroupConfig(options, 'group1', pconfigs=[pconfig])
+        supervisord.options.process_group_configs = [gconfig]
+        process = DummyProcessGroup(gconfig)
+        process.unstopped_processes = [123]
+        supervisord.process_groups = {'group1':process}
+        interface = self._makeOne(supervisord)
+        self._assertRPCError(xmlrpc.Faults.STILL_RUNNING,
+                             interface.removeProcess, 'group1')
+
+
     def test_startProcess_already_started(self):
         from supervisor import xmlrpc
         options = DummyOptions()

src/supervisor/tests/test_supervisorctl.py

@@ -481,14 +481,48 @@ class TestDefaultControllerPlugin(unittest.TestCase):
         supervisor = plugin.ctl.options._server.supervisor
         self.assertEqual(supervisor.processes, ['foo'])
 
-    def test_drop(self):
+    def test_add_already_added(self):
+        plugin = self._makeOne()
+        result = plugin.do_add('ALREADY_ADDED')
+        self.assertEqual(result, None)
+        supervisor = plugin.ctl.options._server.supervisor
+        self.assertEqual(plugin.ctl.stdout.getvalue(),
+                         'ERROR: process group already active\n')
+
+    def test_add_bad_name(self):
+        plugin = self._makeOne()
+        result = plugin.do_add('BAD_NAME')
+        self.assertEqual(result, None)
+        supervisor = plugin.ctl.options._server.supervisor
+        self.assertEqual(plugin.ctl.stdout.getvalue(),
+                         'ERROR: no such process/group: BAD_NAME\n')
+
+    def test_remove(self):
         plugin = self._makeOne()
         supervisor = plugin.ctl.options._server.supervisor
         supervisor.processes = ['foo']
-        result = plugin.do_drop('foo')
+        result = plugin.do_remove('foo')
         self.assertEqual(result, None)
         self.assertEqual(supervisor.processes, [])
 
+    def test_remove_bad_name(self):
+        plugin = self._makeOne()
+        supervisor = plugin.ctl.options._server.supervisor
+        supervisor.processes = ['foo']
+        result = plugin.do_remove('BAD_NAME')
+        self.assertEqual(result, None)
+        self.assertEqual(plugin.ctl.stdout.getvalue(),
+                         'ERROR: no such process/group: BAD_NAME\n')
+
+    def test_remove_still_running(self):
+        plugin = self._makeOne()
+        supervisor = plugin.ctl.options._server.supervisor
+        supervisor.processes = ['foo']
+        result = plugin.do_remove('STILL_RUNNING')
+        self.assertEqual(result, None)
+        self.assertEqual(plugin.ctl.stdout.getvalue(),
+                         'ERROR: process/group still running: STILL_RUNNING\n')
+
     def test_pid(self):
         plugin = self._makeOne()
         result = plugin.do_pid('')

src/supervisor/xmlrpc.py

@@ -43,6 +43,8 @@ class Faults:
     ALREADY_STARTED = 60
     NOT_RUNNING = 70
     SUCCESS = 80
+    ALREADY_ADDED = 90
+    STILL_RUNNING = 91
 
 def getFaultDescription(code):
     for faultname in Faults.__dict__: