[Supervisor-checkins] r904 - in supervisor/trunk: . src/supervisor src/supervisor/tests

[Mike Naberezny]

Author: Mike Naberezny <mike at maintainable.com>
Date: Sat Oct 24 21:05:47 2009
New Revision: 904

Log:
 - Running "supervisorctl shutdown" no longer dumps a Python backtrace
   when it can't connect to supervisord on the expected socket.  Thanks
   to Benjamin Smith for reporting this.



Modified:
   supervisor/trunk/CHANGES.txt
   supervisor/trunk/TODO.txt
   supervisor/trunk/src/supervisor/supervisorctl.py
   supervisor/trunk/src/supervisor/tests/test_supervisorctl.py

Modified: supervisor/trunk/CHANGES.txt
==============================================================================
--- supervisor/trunk/CHANGES.txt	(original)
+++ supervisor/trunk/CHANGES.txt	Sat Oct 24 21:05:47 2009
@@ -47,6 +47,10 @@
 
   - Updated ez_setup.py to one that knows about setuptools 0.6c11.
 
+  - Running "supervisorctl shutdown" no longer dumps a Python backtrace
+    when it can't connect to supervisord on the expected socket.  Thanks
+    to Benjamin Smith for reporting this.
+
 3.0a7 (2009-05-24)
  
   - We now bundle our own patched version of Medusa contributed by Jason

Modified: supervisor/trunk/TODO.txt
==============================================================================
--- supervisor/trunk/TODO.txt	(original)
+++ supervisor/trunk/TODO.txt	Sat Oct 24 21:05:47 2009
@@ -1,7 +1,3 @@
-- Consider adding a friendlier error message to supervisorctl when it can't
-  connect to supervisord.  Currently, it dumps a Python backtrace.  Requested
-  by Benjamin Smith.
-
 - Both the "tail" and "fg" commands in supervisorctl have tests to verify
   their error handling but not actual operation.  We should add some additional 
   tests to verify their operation for completeness.

Modified: supervisor/trunk/src/supervisor/supervisorctl.py
==============================================================================
--- supervisor/trunk/src/supervisor/supervisorctl.py	(original)
+++ supervisor/trunk/src/supervisor/supervisorctl.py	Sat Oct 24 21:05:47 2009
@@ -757,6 +757,7 @@
             really = yesno.lower().startswith('y')
         else:
             really = 1
+
         if really:
             supervisor = self.ctl.get_supervisor()
             try:
@@ -764,6 +765,14 @@
             except xmlrpclib.Fault, e:
                 if e.faultCode == xmlrpc.Faults.SHUTDOWN_STATE:
                     self.ctl.output('ERROR: already shutting down')
+                else:
+                    raise
+            except socket.error, e:
+                if e[0] == errno.ECONNREFUSED:
+                    msg = 'ERROR: %s refused connection (already shut down?)'
+                    self.ctl.output(msg % self.ctl.options.serverurl)
+                else:
+                    raise
             else:
                 self.ctl.output('Shut down')
 

Modified: supervisor/trunk/src/supervisor/tests/test_supervisorctl.py
==============================================================================
--- supervisor/trunk/src/supervisor/tests/test_supervisorctl.py	(original)
+++ supervisor/trunk/src/supervisor/tests/test_supervisorctl.py	Sat Oct 24 21:05:47 2009
@@ -495,21 +495,66 @@
         result = plugin.do_reload('')
         self.assertEqual(result, None)
         self.assertEqual(options._server.supervisor._restarted, True)
-        
-    def test_shutdown_fail(self):
+
+    def test_shutdown(self):
         plugin = self._makeOne()
         options = plugin.ctl.options
-        options._server.supervisor._restartable = False
         result = plugin.do_shutdown('')
         self.assertEqual(result, None)
-        self.assertEqual(options._server.supervisor._shutdown, False)
+        self.assertEqual(options._server.supervisor._shutdown, True)
+        
+    def test_shutdown_catches_xmlrpc_fault_shutdown_state(self):
+        plugin = self._makeOne()
+        from supervisor import xmlrpc
+        import xmlrpclib
+        
+        def raise_fault(*arg, **kw):     
+            raise xmlrpclib.Fault(xmlrpc.Faults.SHUTDOWN_STATE, 'bye')
+        plugin.ctl.options._server.supervisor.shutdown = raise_fault
+
+        result = plugin.do_shutdown('')
+        self.assertEqual(result, None)
+        self.assertEqual(plugin.ctl.stdout.getvalue(), 
+                         'ERROR: already shutting down\n')
 
-    def test_shutdown(self):
+    def test_shutdown_reraises_other_xmlrpc_faults(self):
         plugin = self._makeOne()
-        options = plugin.ctl.options
+        from supervisor import xmlrpc
+        import xmlrpclib
+        
+        def raise_fault(*arg, **kw):     
+            raise xmlrpclib.Fault(xmlrpc.Faults.CANT_REREAD, 'ouch')
+        plugin.ctl.options._server.supervisor.shutdown = raise_fault
+
+        self.assertRaises(xmlrpclib.Fault, 
+                          plugin.do_shutdown, '')
+
+    def test_shutdown_catches_socket_error_ECONNREFUSED(self):
+        plugin = self._makeOne()
+        import socket
+        import errno
+        
+        def raise_fault(*arg, **kw):     
+            raise socket.error(errno.ECONNREFUSED, 'nobody home')
+        plugin.ctl.options._server.supervisor.shutdown = raise_fault
+
         result = plugin.do_shutdown('')
         self.assertEqual(result, None)
-        self.assertEqual(options._server.supervisor._shutdown, True)
+        self.assertEqual(plugin.ctl.stdout.getvalue(), 
+                         'ERROR: http://localhost:92491 refused'
+                         ' connection (already shut down?)\n')
+
+    def test_shutdown_reraises_other_socket_errors(self):
+        plugin = self._makeOne()
+        import socket
+        import errno
+
+        def raise_fault(*arg, **kw):     
+            raise socket.error(errno.EPERM, 'denied')
+        plugin.ctl.options._server.supervisor.shutdown = raise_fault
+
+        self.assertRaises(socket.error, 
+                          plugin.do_shutdown, '')
 
     def test__formatChanges(self):
         plugin = self._makeOne()