[Supervisor-checkins] r827 - in supervisor/branches/chrism-30-merge/src/supervisor: . tests

Chris McDonough chrism at agendaless.com
Sun Nov 23 14:30:44 EST 2008


Author: Chris McDonough <chrism at agendaless.com>
Date: Sun Nov 23 14:30:44 2008
New Revision: 827

Log:
Better error behavior when reread fails due to syntax error in config file.


Modified:
   supervisor/branches/chrism-30-merge/src/supervisor/options.py
   supervisor/branches/chrism-30-merge/src/supervisor/rpcinterface.py
   supervisor/branches/chrism-30-merge/src/supervisor/supervisorctl.py
   supervisor/branches/chrism-30-merge/src/supervisor/tests/base.py
   supervisor/branches/chrism-30-merge/src/supervisor/tests/test_options.py
   supervisor/branches/chrism-30-merge/src/supervisor/tests/test_rpcinterfaces.py
   supervisor/branches/chrism-30-merge/src/supervisor/tests/test_supervisorctl.py
   supervisor/branches/chrism-30-merge/src/supervisor/xmlrpc.py

Modified: supervisor/branches/chrism-30-merge/src/supervisor/options.py
==============================================================================
--- supervisor/branches/chrism-30-merge/src/supervisor/options.py	(original)
+++ supervisor/branches/chrism-30-merge/src/supervisor/options.py	Sun Nov 23 14:30:44 2008
@@ -75,6 +75,9 @@
     pass
 
 class Options:
+    stderr = sys.stderr
+    stdout = sys.stdout
+    exit = sys.exit
 
     uid = gid = None
 
@@ -124,13 +127,13 @@
         if help.find("%s") > 0:
             help = help.replace("%s", self.progname)
         print help,
-        sys.exit(0)
+        self.exit(0)
 
     def usage(self, msg):
         """Print a brief error message to stderr and exit(2)."""
-        sys.stderr.write("Error: %s\n" % str(msg))
-        sys.stderr.write("For help, use %s -h\n" % self.progname)
-        sys.exit(2)
+        self.stderr.write("Error: %s\n" % str(msg))
+        self.stderr.write("For help, use %s -h\n" % self.progname)
+        self.exit(2)
 
     def add(self,
             name=None,                  # attribute name on self
@@ -292,7 +295,7 @@
 
         self.process_config_file()
 
-    def process_config_file(self):
+    def process_config_file(self, do_usage=True):
         # Process config file
         if not hasattr(self.configfile, 'read'):
             self.here = os.path.abspath(os.path.dirname(self.configfile))
@@ -300,7 +303,12 @@
         try:
             self.read_config(self.configfile)
         except ValueError, msg:
-            self.usage(str(msg))
+            if do_usage:
+                # if this is not called from an RPC method, run usage and exit.
+                self.usage(str(msg))
+            else:
+                # if this is called from an RPC method, raise an error
+                raise ValueError(msg)
 
         # Copy config options to attributes of self.  This only fills
         # in options that aren't already set from the command line.
@@ -471,8 +479,8 @@
 
         self.identifier = section.identifier
 
-    def process_config_file(self):
-        Options.process_config_file(self)
+    def process_config_file(self, do_usage=True):
+        Options.process_config_file(self, do_usage=do_usage)
 
         new = self.configroot.supervisord.process_group_configs
         self.process_group_configs = new
@@ -485,7 +493,10 @@
             except (IOError, OSError):
                 raise ValueError("could not find config file %s" % fp)
         parser = UnhosedConfigParser()
-        parser.readfp(fp)
+        try:
+            parser.readfp(fp)
+        except ConfigParser.ParsingError, why:
+            raise ValueError(str(why))
 
         if parser.has_section('include'):
             if not parser.has_option('include', 'files'):
@@ -502,7 +513,10 @@
                 for filename in glob.glob(pattern):
                     self.parse_warnings.append(
                         'Included extra file "%s" during parsing' % filename)
-                    parser.read(filename)
+                    try:
+                        parser.read(filename)
+                    except ConfigParser.ParsingError, why:
+                        raise ValueError(str(why))
 
         sections = parser.sections()
         if not 'supervisord' in sections:
@@ -903,11 +917,11 @@
                 self.logger.info("set current directory: %r"
                                  % self.directory)
         os.close(0)
-        sys.stdin = sys.__stdin__ = open("/dev/null")
+        self.stdin = sys.stdin = sys.__stdin__ = open("/dev/null")
         os.close(1)
-        sys.stdout = sys.__stdout__ = open("/dev/null", "w")
+        self.stdout = sys.stdout = sys.__stdout__ = open("/dev/null", "w")
         os.close(2)
-        sys.stderr = sys.__stderr__ = open("/dev/null", "w")
+        self.stderr = sys.stderr = sys.__stderr__ = open("/dev/null", "w")
         os.setsid()
         os.umask(self.umask)
         # XXX Stevens, in his Advanced Unix book, section 13.3 (page

Modified: supervisor/branches/chrism-30-merge/src/supervisor/rpcinterface.py
==============================================================================
--- supervisor/branches/chrism-30-merge/src/supervisor/rpcinterface.py	(original)
+++ supervisor/branches/chrism-30-merge/src/supervisor/rpcinterface.py	Sun Nov 23 14:30:44 2008
@@ -169,7 +169,11 @@
         @return boolean result  always return True unless error
         """
         self._update('reloadConfig')
-        self.supervisord.options.process_config_file()
+        try:
+            self.supervisord.options.process_config_file(do_usage=False)
+        except ValueError, msg:
+            raise RPCError(Faults.CANT_REREAD, msg)
+            
         added, changed, removed = self.supervisord.diff_to_active()
 
         added = [group.name for group in added]

Modified: supervisor/branches/chrism-30-merge/src/supervisor/supervisorctl.py
==============================================================================
--- supervisor/branches/chrism-30-merge/src/supervisor/supervisorctl.py	(original)
+++ supervisor/branches/chrism-30-merge/src/supervisor/supervisorctl.py	Sun Nov 23 14:30:44 2008
@@ -789,7 +789,7 @@
             for name in sorted(changedict):
                 self.ctl.output("%s: %s" % (name, changedict[name]))
         else:
-            self.ctl.output("No config updates to proccesses")
+            self.ctl.output("No config updates to processes")
 
     def _formatConfigInfo(self, configinfo):
         if configinfo['group'] == configinfo['name']:
@@ -831,7 +831,11 @@
             result = supervisor.reloadConfig()
         except xmlrpclib.Fault, e:
             if e.faultCode == xmlrpc.Faults.SHUTDOWN_STATE:
-                self.ctl.output('ERROR: already shutting down')
+                self.ctl.output('ERROR: supervisor shutting down')
+            elif e.faultCode == xmlrpc.Faults.CANT_REREAD:
+                self.ctl.output('ERROR: %s' % e.faultString)
+            else:
+                raise
         else:
             self._formatChanges(result[0])
 

Modified: supervisor/branches/chrism-30-merge/src/supervisor/tests/base.py
==============================================================================
--- supervisor/branches/chrism-30-merge/src/supervisor/tests/base.py	(original)
+++ supervisor/branches/chrism-30-merge/src/supervisor/tests/base.py	Sun Nov 23 14:30:44 2008
@@ -79,7 +79,7 @@
         self.realizeargs = args
         self.realizekw = kw
 
-    def process_config_file(self):
+    def process_config_file(self, do_usage=True):
         pass
 
     def cleanup_fds(self):

Modified: supervisor/branches/chrism-30-merge/src/supervisor/tests/test_options.py
==============================================================================
--- supervisor/branches/chrism-30-merge/src/supervisor/tests/test_options.py	(original)
+++ supervisor/branches/chrism-30-merge/src/supervisor/tests/test_options.py	Sun Nov 23 14:30:44 2008
@@ -20,7 +20,7 @@
 
 class OptionTests(unittest.TestCase):
 
-    def _makeOptions(self):
+    def _makeOptions(self, read_error=False):
         from cStringIO import StringIO
         from supervisor.options import Options
         from supervisor.datatypes import integer
@@ -28,12 +28,15 @@
         class MyOptions(Options):
             master = {
                 'other': 41 }
-            def __init__(self):
+            def __init__(self, read_error=read_error):
+                self.read_error = read_error
                 Options.__init__(self)
                 class Foo(object): pass
                 self.configroot = Foo()
 
             def read_config(self, fp):
+                if self.read_error:
+                    raise ValueError(self.read_error)
                 # Pretend we read it from file:
                 self.configroot.__dict__.update(self.default_map)
                 self.configroot.__dict__.update(self.master)
@@ -73,6 +76,24 @@
         options.process_config_file()
         self.assertEquals(options.other, 42)
 
+    def test_config_reload_do_usage_false(self):
+        options = self._makeOptions(read_error='error')
+        self.assertRaises(ValueError, options.process_config_file,
+                          False)
+
+    def test_config_reload_do_usage_true(self):
+        options = self._makeOptions(read_error='error')
+        from StringIO import StringIO
+        L = []
+        def exit(num):
+            L.append(num)
+        options.stderr = options.stdout = StringIO()
+        options.exit = exit
+        options.configroot.anoption = 1
+        options.configroot.other = 1
+        options.process_config_file(True)
+        self.assertEqual(L, [2])
+
     def test__set(self):
         from supervisor.options import Options
         options = Options()

Modified: supervisor/branches/chrism-30-merge/src/supervisor/tests/test_rpcinterfaces.py
==============================================================================
--- supervisor/branches/chrism-30-merge/src/supervisor/tests/test_rpcinterfaces.py	(original)
+++ supervisor/branches/chrism-30-merge/src/supervisor/tests/test_rpcinterfaces.py	Sun Nov 23 14:30:44 2008
@@ -232,6 +232,16 @@
         value = interface.reloadConfig()
         self.assertEqual(value, [[['added'], ['changed'], ['dropped']]])
 
+    def test_reloadConfig_process_config_file_raises_ValueError(self):
+        from supervisor import xmlrpc
+        options = DummyOptions()
+        def raise_exc(*arg, **kw):
+            raise ValueError('foo')
+        options.process_config_file = raise_exc
+        supervisord = DummySupervisor(options)
+        interface = self._makeOne(supervisord)
+        self._assertRPCError(xmlrpc.Faults.CANT_REREAD, interface.reloadConfig)
+
     def test_addProcessGroup(self):
         from supervisor.supervisord import Supervisor
         from supervisor import xmlrpc

Modified: supervisor/branches/chrism-30-merge/src/supervisor/tests/test_supervisorctl.py
==============================================================================
--- supervisor/branches/chrism-30-merge/src/supervisor/tests/test_supervisorctl.py	(original)
+++ supervisor/branches/chrism-30-merge/src/supervisor/tests/test_supervisorctl.py	Sun Nov 23 14:30:44 2008
@@ -486,6 +486,17 @@
         self.assertEqual(result, None)
         self.assertEqual(calls[0], [['added'], ['changed'], ['removed']])
 
+    def test_reread_Fault(self):
+        plugin = self._makeOne()
+        from supervisor import xmlrpc
+        import xmlrpclib
+        def raise_fault(*arg, **kw):
+            raise xmlrpclib.Fault(xmlrpc.Faults.CANT_REREAD, 'cant')
+        plugin.ctl.options._server.supervisor.reloadConfig = raise_fault
+        plugin.do_reread(None)
+        self.assertEqual(plugin.ctl.stdout.getvalue(),
+                         'ERROR: cant\n')
+
     def test__formatConfigInfo(self):
         info = { 'group': 'group1',
                  'name': 'process1',
@@ -649,7 +660,6 @@
 
     def test_update_removed_procs(self):
         from supervisor import xmlrpc
-        import xmlrpclib
 
         plugin = self._makeOne()
         supervisor = plugin.ctl.options._server.supervisor

Modified: supervisor/branches/chrism-30-merge/src/supervisor/xmlrpc.py
==============================================================================
--- supervisor/branches/chrism-30-merge/src/supervisor/xmlrpc.py	(original)
+++ supervisor/branches/chrism-30-merge/src/supervisor/xmlrpc.py	Sun Nov 23 14:30:44 2008
@@ -45,6 +45,7 @@
     SUCCESS = 80
     ALREADY_ADDED = 90
     STILL_RUNNING = 91
+    CANT_REREAD = 92
 
 def getFaultDescription(code):
     for faultname in Faults.__dict__:


More information about the Supervisor-checkins mailing list