Testing tweaks, move childlogdir responsibility to supervisord class.

rpc.py

@@ -171,10 +171,6 @@ class SupervisorNamespaceRPCInterface:
 
     def __init__(self, supervisord):
         self.supervisord = supervisord
-        self.cache = {}
-        self._update('__init__')
-        if not self.supervisord.options.nocleanup:
-            self._clear_childlogdir()
 
     def _update(self, text):
         self.update_text = text # for unit tests, mainly
@@ -184,59 +180,6 @@ class SupervisorNamespaceRPCInterface:
         if state == SupervisorStates.SHUTDOWN:
             raise RPCError('SHUTDOWN_STATE')
 
-    def _clear_childlogdir(self):
-        options = self.supervisord.options
-        childlogdir = options.childlogdir
-        fnre = re.compile(r'.+?---\d{1,11}-%s-\S+?\.xlog' % options.identifier)
-        try:
-            filenames = os.listdir(childlogdir)
-        except (IOError, OSError):
-            options.logger.info('Could not clear childlog dir')
-            return
-        
-        for filename in filenames:
-            if fnre.match(filename):
-                pathname = os.path.join(childlogdir, filename)
-                try:
-                    os.remove(pathname)
-                except (os.error, IOError):
-                    options.logger.info('Failed to clean up %r' % pathname)
-
-    def _readFile(self, filename, offset, length):
-        """ Read length bytes from the file named by filename starting at
-        offset """
-
-        absoffset = abs(offset)
-        abslength = abs(length)
-
-        try:
-            f = open(filename, 'rb')
-            if absoffset != offset:
-                # negative offset returns offset bytes from tail of the file
-                if length:
-                    raise ValueError('BAD_ARGUMENTS')
-                f.seek(0, 2)
-                sz = f.tell()
-                pos = int(sz - absoffset)
-                f.seek(pos)
-                data = f.read(absoffset)
-            else:
-                if abslength != length:
-                    raise ValueError('BAD_ARGUMENTS')
-                if length == 0:
-                    f.seek(offset)
-                    data = f.read()
-                else:
-                    sz = f.seek(offset)
-                    data = f.read(length)
-        except (os.error, IOError):
-            # XXX don't set fatal state, bad offset or length
-            # shouldn't cause fatal
-            #msg = 'FATAL: failed reading log file %r' % filename
-            raise ValueError('FAILED')
-
-        return data
-
     # RPC API methods
 
     def getVersion(self):
@@ -285,7 +228,7 @@ class SupervisorNamespaceRPCInterface:
             raise RPCError('SUPER_READ_NO_FILE')
 
         try:
-            return self._readFile(logfile, offset, length)
+            return _readFile(logfile, offset, length)
         except ValueError, inst:
             why = inst.args[0]
             raise RPCError('SUPER_READ_' + why)
@@ -317,9 +260,7 @@ class SupervisorNamespaceRPCInterface:
 
         @return boolean result always returns True unless error
         """
-        # we don't want to call _update here.
-        if self.supervisord.get_state() == SupervisorStates.SHUTDOWN:
-            raise RPCError('SHUTDOWN_STATE')
+        self._update('shutdown')
         
         self.supervisord.mood = -1
         return True
@@ -329,8 +270,7 @@ class SupervisorNamespaceRPCInterface:
 
         @return boolean result  always return True unless error
         """
-        if self.supervisord.get_state() == SupervisorStates.SHUTDOWN:
-            raise RPCError('SHUTDOWN_STATE')
+        self._update('restart')
         
         self.supervisord.mood = 0
         return True
@@ -588,7 +528,7 @@ class SupervisorNamespaceRPCInterface:
             raise RPCError('READ_NO_FILE')
 
         try:
-            return self._readFile(logfile, offset, length)
+            return _readFile(logfile, offset, length)
         except ValueError, inst:
             why = inst.args[0]
             raise RPCError('READ_' + why)
@@ -828,8 +768,8 @@ def traverse(ob, method, params):
     path = method.split('.')
     for name in path:
         if name.startswith('_'):
-            # security (don't allow things like _readFile to be called
-            # remotely)
+            # security (don't allow things that start with an underscore to
+            # be called remotely)
             raise RPCError('UNKNOWN_METHOD')
         ob = getattr(ob, name, None)
         if ob is None:
@@ -840,3 +780,35 @@ def traverse(ob, method, params):
     except TypeError:
         raise RPCError('INCORRECT_PARAMETERS')
 
+def _readFile(filename, offset, length):
+    """ Read length bytes from the file named by filename starting at
+    offset """
+
+    absoffset = abs(offset)
+    abslength = abs(length)
+
+    try:
+        f = open(filename, 'rb')
+        if absoffset != offset:
+            # negative offset returns offset bytes from tail of the file
+            if length:
+                raise ValueError('BAD_ARGUMENTS')
+            f.seek(0, 2)
+            sz = f.tell()
+            pos = int(sz - absoffset)
+            f.seek(pos)
+            data = f.read(absoffset)
+        else:
+            if abslength != length:
+                raise ValueError('BAD_ARGUMENTS')
+            if length == 0:
+                f.seek(offset)
+                data = f.read()
+            else:
+                sz = f.seek(offset)
+                data = f.read(length)
+    except (os.error, IOError):
+        raise ValueError('FAILED')
+
+    return data
+

src/supervisor/supervisord.py

@@ -458,8 +458,29 @@ class Supervisor:
         for msg in held_messages:
             self.options.logger.info(msg)
             
+        if not self.options.nocleanup:
+            self._clear_childlogdir()
+
         self.run(test)
 
+    def _clear_childlogdir(self):
+        options = self.options
+        childlogdir = options.childlogdir
+        fnre = re.compile(r'.+?---\d{1,11}-%s-\S+?\.xlog' % options.identifier)
+        try:
+            filenames = os.listdir(childlogdir)
+        except (IOError, OSError):
+            options.logger.info('Could not clear childlog dir')
+            return
+        
+        for filename in filenames:
+            if fnre.match(filename):
+                pathname = os.path.join(childlogdir, filename)
+                try:
+                    os.remove(pathname)
+                except (os.error, IOError):
+                    options.logger.info('Failed to clean up %r' % pathname)
+
     def get_state(self):
         if self.mood <= 0:
             return SupervisorStates.SHUTDOWN

src/supervisor/tests.py

@@ -206,7 +206,9 @@ class MainXMLRPCInterfaceTests(TestBase):
             traverse(interface, 'supervisor.getIdentification', []),
             'supervisor')
             
-def makeExecutable(file, substitutions={}):
+def makeExecutable(file, substitutions=None):
+    if substitutions is None:
+        substitutions = {}
     data = open(file).read()
     last = os.path.split(file)[1]
 
@@ -235,15 +237,13 @@ class SupervisorNamespaceXMLRPCInterfaceTests(TestBase):
     def _makeOne(self, *args, **kw):
         return self._getTargetClass()(*args, **kw)
 
-    def test_ctor(self):
-        supervisord = DummySupervisor()
-        interface = self._makeOne(supervisord)
-
     def test_update(self):
         supervisord = DummySupervisor()
         interface = self._makeOne(supervisord)
         interface._update('foo')
         self.assertEqual(interface.update_text, 'foo')
+        supervisord.state = SupervisorStates.SHUTDOWN
+        self._assertRPCError('SHUTDOWN_STATE', interface._update, 'foo')
 
     def test_getVersion(self):
         supervisord = DummySupervisor()
@@ -270,6 +270,88 @@ class SupervisorNamespaceXMLRPCInterfaceTests(TestBase):
         self.assertEqual(stateinfo['statename'], statename)
         self.assertEqual(interface.update_text, 'getState')
 
+    def test_readLog_unreadable(self):
+        supervisord = DummySupervisor()
+        interface = self._makeOne(supervisord)
+        self._assertRPCError('SUPER_READ_NO_FILE', interface.readLog,
+                             offset=0, length=1)
+
+    def test_readLog_badargs(self):
+        supervisord = DummySupervisor()
+        interface = self._makeOne(supervisord)
+        try:
+            logfile = supervisord.options.logfile
+            f = open(logfile, 'w+')
+            f.write('x' * 2048)
+            f.close()
+            self._assertRPCError('SUPER_READ_BAD_ARGUMENTS',
+                                 interface.readLog, offset=-1, length=1)
+            self._assertRPCError('SUPER_READ_BAD_ARGUMENTS',
+                                 interface.readLog, offset=-1,
+                                 length=-1)
+        finally:
+            os.remove(logfile)
+
+    def test_readLog(self):
+        supervisord = DummySupervisor()
+        interface = self._makeOne(supervisord)
+        logfile = supervisord.options.logfile
+        try:
+            f = open(logfile, 'w+')
+            f.write('x' * 2048)
+            f.write('y' * 2048)
+            f.close()
+            data = interface.readLog(offset=0, length=0)
+            self.assertEqual(interface.update_text, 'readLog')
+            self.assertEqual(data, ('x' * 2048) + ('y' * 2048))
+            data = interface.readLog(offset=2048, length=0)
+            self.assertEqual(data, 'y' * 2048)
+            data = interface.readLog(offset=0, length=2048)
+            self.assertEqual(data, 'x' * 2048)
+            data = interface.readLog(offset=-4, length=0)
+            self.assertEqual(data, 'y' * 4)
+        finally:
+            os.remove(logfile)
+
+    def test_clearLog_unreadable(self):
+        supervisord = DummySupervisor()
+        interface = self._makeOne(supervisord)
+        self._assertRPCError('SUPER_CLEAR_NO_FILE', interface.clearLog)
+
+    def test_clearLog(self):
+        supervisord = DummySupervisor()
+        interface = self._makeOne(supervisord)
+        logfile = supervisord.options.logfile
+        try:
+            f = open(logfile, 'w+')
+            f.write('x')
+            f.close()
+            result = interface.clearLog()
+            self.assertEqual(interface.update_text, 'clearLog')
+            self.assertEqual(result, True)
+            self.failIf(os.path.exists(logfile))
+        finally:
+            try:
+                os.remove(logfile)
+            except:
+                pass
+
+        self.assertEqual(supervisord.options.logger.handlers[0].reopened, True)
+
+    def test_shutdown(self):
+        supervisord = DummySupervisor()
+        interface = self._makeOne(supervisord)
+        value = interface.shutdown()
+        self.assertEqual(value, True)
+        self.assertEqual(supervisord.mood, -1)
+
+    def test_restart(self):
+        supervisord = DummySupervisor()
+        interface = self._makeOne(supervisord)
+        value = interface.restart()
+        self.assertEqual(value, True)
+        self.assertEqual(supervisord.mood, 0)
+
     def test_startProcess_already_started(self):
         options = DummyOptions()
         config = DummyPConfig('foo', '/bin/foo', autostart=False)
@@ -356,99 +438,6 @@ class SupervisorNamespaceXMLRPCInterfaceTests(TestBase):
         self.assertEqual(process2.spawned, True)
         
 
-    def test_readLog_unreadable(self):
-        supervisord = DummySupervisor()
-        interface = self._makeOne(supervisord)
-        self._assertRPCError('SUPER_READ_NO_FILE', interface.readLog,
-                             offset=0, length=1)
-
-    def test_readLog_badargs(self):
-        supervisord = DummySupervisor()
-        interface = self._makeOne(supervisord)
-        try:
-            logfile = supervisord.options.logfile
-            f = open(logfile, 'w+')
-            f.write('x' * 2048)
-            f.close()
-            self._assertRPCError('SUPER_READ_BAD_ARGUMENTS',
-                                 interface.readLog, offset=-1, length=1)
-            self._assertRPCError('SUPER_READ_BAD_ARGUMENTS',
-                                 interface.readLog, offset=-1,
-                                 length=-1)
-        finally:
-            os.remove(logfile)
-
-    def test_readLog(self):
-        supervisord = DummySupervisor()
-        interface = self._makeOne(supervisord)
-        logfile = supervisord.options.logfile
-        try:
-            f = open(logfile, 'w+')
-            f.write('x' * 2048)
-            f.write('y' * 2048)
-            f.close()
-            data = interface.readLog(offset=0, length=0)
-            self.assertEqual(interface.update_text, 'readLog')
-            self.assertEqual(data, ('x' * 2048) + ('y' * 2048))
-            data = interface.readLog(offset=2048, length=0)
-            self.assertEqual(data, 'y' * 2048)
-            data = interface.readLog(offset=0, length=2048)
-            self.assertEqual(data, 'x' * 2048)
-            data = interface.readLog(offset=-4, length=0)
-            self.assertEqual(data, 'y' * 4)
-        finally:
-            os.remove(logfile)
-
-    def test_clearLog_unreadable(self):
-        supervisord = DummySupervisor()
-        interface = self._makeOne(supervisord)
-        self._assertRPCError('SUPER_CLEAR_NO_FILE', interface.clearLog)
-
-    def test_shutdown(self):
-        supervisord = DummySupervisor()
-        interface = self._makeOne(supervisord)
-        value = interface.shutdown()
-        self.assertEqual(value, True)
-        self.assertEqual(supervisord.mood, -1)
-
-    def test_restart(self):
-        supervisord = DummySupervisor()
-        interface = self._makeOne(supervisord)
-        value = interface.restart()
-        self.assertEqual(value, True)
-        self.assertEqual(supervisord.mood, 0)
-
-    def test_readFile_failed(self):
-        supervisord = DummySupervisor()
-        interface = self._makeOne(supervisord)
-        logfile = supervisord.options.logfile
-        try:
-            interface._readFile('/notthere', 0, 10)
-        except ValueError, inst:
-            self.assertEqual(inst.args[0], 'FAILED')
-        else:
-            raise AssertionError("Didn't raise")
-
-    def test_clearLog(self):
-        supervisord = DummySupervisor()
-        interface = self._makeOne(supervisord)
-        logfile = supervisord.options.logfile
-        try:
-            f = open(logfile, 'w+')
-            f.write('x')
-            f.close()
-            result = interface.clearLog()
-            self.assertEqual(interface.update_text, 'clearLog')
-            self.assertEqual(result, True)
-            self.failIf(os.path.exists(logfile))
-        finally:
-            try:
-                os.remove(logfile)
-            except:
-                pass
-
-        self.assertEqual(supervisord.options.logger.handlers[0].reopened, True)
-
     def test_stopProcess_badname(self):
         supervisord = DummySupervisor()
         interface = self._makeOne(supervisord)
@@ -644,6 +633,19 @@ class SupervisorNamespaceXMLRPCInterfaceTests(TestBase):
         interface.clearProcessLog('foo')
         self.assertEqual(process.logsremoved, True)
 
+    def test_readFile_failed(self):
+        from rpc import _readFile
+        supervisord = DummySupervisor()
+        interface = self._makeOne(supervisord)
+        logfile = supervisord.options.logfile
+        try:
+            _readFile('/notthere', 0, 10)
+        except ValueError, inst:
+            self.assertEqual(inst.args[0], 'FAILED')
+        else:
+            raise AssertionError("Didn't raise")
+
+
 class SystemNamespaceXMLRPCInterfaceTests(TestBase):
     def _getTargetClass(self):
         return rpc.SystemNamespaceRPCInterface