Revert rotating log handler changes. Closes #10, 121, 130, 184.

This reverts the changes in commits d2bc685
and aabd376.

CHANGES.txt

@@ -8,6 +8,15 @@ Next release
   ``supervisord.conf`` file by supplying the connection information in
   command line options.  Patch by Jens Rantil.
 
+- Reverted a change to logging introduced in 3.0b1 that was intended to allow
+  multiple processes to log to the same file with the rotating log handler.
+  The implementation caused supervisord to crash during reload and to leak
+  file handles.  Also, since log rotation options are given on a per-program
+  basis, impossible configurations could be created (conflicting rotation
+  options for the same file).  Given this and that supervisord now has syslog
+  support, it was decided to remove this feature.  A warning was added to the
+  documentation that two processes may not log to the same file.
+
 3.0b1 (2012-09-10)
 ------------------
 

docs/configuration.rst

@@ -780,6 +780,12 @@ where specified.
   ``process_num``, ``program_name``, and ``here`` (the directory of the
   supervisord config file).
 
+  .. note::
+
+     It is not possible for two processes to share a single log file
+     (``stdout_logfile``) when rotation (``stdout_logfile_maxbytes``)
+     is enabled.  This will result in the file being corrupted.
+
   *Default*: ``AUTO``
 
   *Required*:  No.
@@ -843,6 +849,12 @@ where specified.
   true.  Accepts the same value types as ``stdout_logfile`` and may
   contain the same Python string expressions.
 
+  .. note::
+
+     It is not possible for two processes to share a single log file
+     (``stderr_logfile``) when rotation (``stderr_logfile_maxbytes``)
+     is enabled.  This will result in the file being corrupted.
+
   *Default*: ``AUTO``
 
   *Required*:  No.

supervisor/loggers.py

@@ -50,7 +50,7 @@ def getLevelNumByDescription(description):
     num = getattr(LevelsByDescription, description, None)
     return num
 
-class Handler(object):
+class Handler:
     fmt = '%(message)s'
     level = LevelsByName.INFO
     def setFormat(self, fmt):
@@ -93,6 +93,7 @@ def handleError(self, record):
 class FileHandler(Handler):
     """File handler which supports reopening of logs.
     """
+
     def __init__(self, filename, mode="a"):
         self.stream = open(filename, mode)
         self.baseFilename = filename
@@ -144,9 +145,6 @@ def clear(self):
         self.buf = ''
 
 class RotatingFileHandler(FileHandler):
-
-    open_streams = {}
-
     def __init__(self, filename, mode='a', maxBytes=512*1024*1024,
                  backupCount=10):
         """
@@ -171,38 +169,12 @@ def __init__(self, filename, mode='a', maxBytes=512*1024*1024,
         """
         if maxBytes > 0:
             mode = 'a' # doesn't make sense otherwise!
-        self.mode = mode
-        self.baseFilename = filename
-        self.stream = self.stream or open(filename, mode)
-
+        FileHandler.__init__(self, filename, mode)
         self.maxBytes = maxBytes
         self.backupCount = backupCount
         self.counter = 0
         self.every = 10
 
-    class _stream(object):
-        """
-        Descriptor for managing open filehandles so that only one
-        filehandle per file path ever receives logging.
-        """
-        def __get__(self, obj, objtype):
-            """
-            Return open filehandle or None
-            """
-            return objtype.open_streams.get(obj.baseFilename)
-
-        def __set__(self, obj, stream):
-            """
-            Set open filehandle for filename defined on the
-            RotatingFileHandler
-            """
-            obj.open_streams[obj.baseFilename] = stream
-
-    stream = _stream()
-
-    def close(self):
-        if self.stream: self.stream.close()
-
     def emit(self, record):
         """
         Emit a record.

supervisor/tests/test_loggers.py

@@ -138,41 +138,6 @@ def test_ctor(self):
         self.assertEqual(handler.maxBytes, 512*1024*1024)
         self.assertEqual(handler.backupCount, 10)
 
-    def test_emit_tracks_correct_file_for_multiple_handlers(self):
-        """
-        Rollovers should roll for all handlers of the same file.
-
-        When more than one process logs to a singlefile, we want to
-        make sure that files get rotated properly.
-
-        When the file rotates, all handlers should start writing to
-        the file specified by handler.baseFilename.
-        """
-        handler1 = self._makeOne(self.filename, maxBytes=10, backupCount=2)
-        handler2 = self._makeOne(self.filename, maxBytes=10, backupCount=2)
-        record = self._makeLogRecord('a' * 4)
-        handler1.emit(record) #4 bytes
-        handler2.emit(record) #8 bytes
-        self.assertFalse(os.path.exists(self.filename + '.1'))
-        handler1.emit(record) #12 bytes
-        self.assertTrue(os.path.exists(self.filename + '.1'))
-        self.assertTrue(handler1.stream == handler2.stream)
-        new_record = self._makeLogRecord("NEW")
-        handler2.emit(new_record)
-        self.assertTrue(open(self.filename).read().endswith("NEW"))
-        handler1.emit(record)
-        self.assertTrue(open(self.filename).read().endswith("aaaa"))
-        handler2.emit(new_record)
-        self.assertTrue(open(self.filename).read().endswith(""))
-
-    def test_reopen_raises(self):
-        handler = self._makeOne(self.filename)
-        stream = DummyStream()
-        handler.baseFilename = os.path.join(self.basedir, 'notthere', 'a.log')
-        handler.open_streams[handler.baseFilename] = stream
-        self.assertRaises(IOError, handler.reopen)
-        self.assertEqual(stream.closed, True)
-
     def test_emit_does_rollover(self):
         handler = self._makeOne(self.filename, maxBytes=10, backupCount=2)
         record = self._makeLogRecord('a' * 4)