r47072 - review comments from wsanchez

Author: hawkowl
Date: Fri Mar 25 08:13:29 2016
New Revision: 47072

Modified:
   branches/tlogger-twistd-8235/twisted/application/app.py
   branches/tlogger-twistd-8235/twisted/internet/protocol.py
   branches/tlogger-twistd-8235/twisted/logger/__init__.py
   branches/tlogger-twistd-8235/twisted/logger/_logger.py
   branches/tlogger-twistd-8235/twisted/test/test_twistd.py

Log:
review comments from wsanchez

Modified: branches/tlogger-twistd-8235/twisted/application/app.py
==============================================================================
--- branches/tlogger-twistd-8235/twisted/application/app.py	(original)
+++ branches/tlogger-twistd-8235/twisted/application/app.py	Fri Mar 25 08:13:29 2016
@@ -19,8 +19,6 @@
 from twisted.internet import defer
 from twisted.persisted import sob
 from twisted.python import runtime, log, usage, failure, util, logfile
-from twisted.logger import (globalLogBeginner, LegacyLogObserverWrapper,
-                            _logFor, ILogObserver, globalLogPublisher)
 from twisted.python.reflect import qual, namedAny
 
 # Expose the new implementation of installReactor at the old location.
@@ -184,7 +182,7 @@
         if self._observerFactory is not None:
             observer = self._observerFactory()
         else:
-            observer = application.getComponent(ILogObserver, None)
+            observer = application.getComponent(logger.ILogObserver, None)
             if observer is None:
                 # If there's no new ILogObserver, try the legacy one
                 observer = application.getComponent(log.ILogObserver, None)
@@ -193,19 +191,25 @@
             observer = self._getLogObserver()
         self._observer = observer
 
-        if ILogObserver.providedBy(self._observer):
+        if logger.ILogObserver.providedBy(self._observer):
             observers = [self._observer]
+        elif log.ILogObserver.providedBy(self._observer):
+            observers = [logger.LegacyLogObserverWrapper(self._observer)]
         else:
             warnings.warn(
-                ("Using legacy log observer factories in "
+                ("Passing a logger factory which makes log observers which do "
+                 "not implement twisted.logger.ILogObserver or "
+                 "twisted.python.log.ILogObserver to "
                  "twisted.application.app.AppLogger was deprecated in "
                  "Twisted 16.1. Please use a factory that produces "
-                 "twisted.logger.ILogObserver implementing objects instead."),
+                 "twisted.logger.ILogObserver (or the legacy "
+                 "twisted.python.log.ILogObserver) implementing objects "
+                 "instead."),
                 DeprecationWarning,
                 stacklevel=2)
-            observers = [LegacyLogObserverWrapper(self._observer)]
+            observers = [logger.LegacyLogObserverWrapper(self._observer)]
 
-        globalLogBeginner.beginLoggingTo(observers)
+        logger.globalLogBeginner.beginLoggingTo(observers)
         self._initialLog()
 
 
@@ -214,11 +218,12 @@
         Print twistd start log message.
         """
         from twisted.internet import reactor
-        _logFor(self).info("twistd {version} ({exe} {pyVersion}) starting up.",
+        logger._loggerFor(self).info(
+            "twistd {version} ({exe} {pyVersion}) starting up.",
             version=copyright.version, exe=sys.executable,
-                           pyVersion=runtime.shortPythonVersion())
-        _logFor(self).info('reactor class: {reactor}.',
-                           reactor=qual(reactor.__class__))
+            pyVersion=runtime.shortPythonVersion())
+        logger._loggerFor(self).info('reactor class: {reactor}.',
+                                     reactor=qual(reactor.__class__))
 
 
     def _getLogObserver(self):
@@ -237,9 +242,9 @@
         """
         Remove all log observers previously set up by L{AppLogger.start}.
         """
-        _logFor(self).info("Server Shut Down.")
+        logger._loggerFor(self).info("Server Shut Down.")
         if self._observer is not None:
-            globalLogPublisher.removeObserver(self._observer)
+            logger.globalLogPublisher.removeObserver(self._observer)
             self._observer = None
 
 

Modified: branches/tlogger-twistd-8235/twisted/internet/protocol.py
==============================================================================
--- branches/tlogger-twistd-8235/twisted/internet/protocol.py	(original)
+++ branches/tlogger-twistd-8235/twisted/internet/protocol.py	Fri Mar 25 08:13:29 2016
@@ -16,7 +16,7 @@
 
 from twisted.python import log, failure, components
 from twisted.internet import interfaces, error, defer
-from twisted.logger import _logFor
+from twisted.logger import _loggerFor
 
 
 @implementer(interfaces.IProtocolFactory, interfaces.ILoggingContext)
@@ -69,8 +69,8 @@
         """
         if not self.numPorts:
             if self.noisy:
-                _logFor(self).info("Starting factory {factory!r}",
-                                   factory=self)
+                _loggerFor(self).info("Starting factory {factory!r}",
+                                      factory=self)
             self.startFactory()
         self.numPorts = self.numPorts + 1
 
@@ -86,8 +86,8 @@
         self.numPorts = self.numPorts - 1
         if not self.numPorts:
             if self.noisy:
-                _logFor(self).info("Stopping factory {factory!r}",
-                                   factory=self)
+                _loggerFor(self).info("Stopping factory {factory!r}",
+                                      factory=self)
             self.stopFactory()
 
     def startFactory(self):

Modified: branches/tlogger-twistd-8235/twisted/logger/__init__.py
==============================================================================
--- branches/tlogger-twistd-8235/twisted/logger/__init__.py	(original)
+++ branches/tlogger-twistd-8235/twisted/logger/__init__.py	Fri Mar 25 08:13:29 2016
@@ -54,7 +54,7 @@
     "extractField",
 
     # From ._logger
-    "Logger",
+    "Logger", "_loggerFor",
 
     # From ._observer
     "ILogObserver", "LogPublisher",
@@ -94,7 +94,7 @@
     formatEvent, formatEventAsClassicLogText, formatTime, timeFormatRFC3339,
 )
 
-from ._logger import Logger
+from ._logger import Logger, _loggerFor
 
 from ._observer import ILogObserver, LogPublisher
 
@@ -121,6 +121,3 @@
     eventAsJSON, eventFromJSON,
     jsonFileLogObserver, eventsFromJSONLogFile
 )
-
-_log = Logger()
-_logFor = lambda _:_log.__get__(_, _.__class__)

Modified: branches/tlogger-twistd-8235/twisted/logger/_logger.py
==============================================================================
--- branches/tlogger-twistd-8235/twisted/logger/_logger.py	(original)
+++ branches/tlogger-twistd-8235/twisted/logger/_logger.py	Fri Mar 25 08:13:29 2016
@@ -256,3 +256,8 @@
             later execution.
         """
         self.emit(LogLevel.critical, format, **kwargs)
+
+
+
+_log = Logger()
+_loggerFor = lambda obj:_log.__get__(obj, obj.__class__)

Modified: branches/tlogger-twistd-8235/twisted/test/test_twistd.py
==============================================================================
--- branches/tlogger-twistd-8235/twisted/test/test_twistd.py	(original)
+++ branches/tlogger-twistd-8235/twisted/test/test_twistd.py	Fri Mar 25 08:13:29 2016
@@ -1384,9 +1384,37 @@
         self.assertIdentical(logger._observer, None)
 
 
-    def test_legacyObserversDeprecated(self):
+    def test_legacyObservers(self):
         """
-        L{app.AppLogger} using a legacy logger observer is deprecated.
+        L{app.AppLogger} using a legacy logger observer still works, wrapping
+        it in a compat shim.
+        """
+        logs = []
+        logger = app.AppLogger({})
+
+        @implementer(LegacyILogObserver)
+        class LoggerObserver(object):
+            """
+            An observer which implements the legacy L{LegacyILogObserver}.
+            """
+            def __call__(self, x):
+                """
+                Add C{x} to the logs list.
+                """
+                logs.append(x)
+
+        logger._observerFactory = lambda: LoggerObserver()
+        logger.start(Componentized())
+
+        self.assertIn("starting up", textFromEventDict(logs[0]))
+        warnings = self.flushWarnings(
+            [self.test_legacyObservers])
+        self.assertEqual(len(warnings), 0)
+
+
+    def test_unmarkedObserversDeprecated(self):
+        """
+        L{app.AppLogger} using a logger observer which
         """
         logs = []
         logger = app.AppLogger({})
@@ -1396,14 +1424,17 @@
         self.assertIn("starting up", textFromEventDict(logs[0]))
 
         warnings = self.flushWarnings(
-            [self.test_legacyObserversDeprecated])
+            [self.test_unmarkedObserversDeprecated])
         self.assertEqual(len(warnings), 1)
         self.assertEqual(warnings[0]["message"],
-                         ("Using legacy log observer factories in "
+                         ("Passing a logger factory which makes log observers "
+                          "which do not implement twisted.logger.ILogObserver "
+                          "or twisted.python.log.ILogObserver to "
                           "twisted.application.app.AppLogger was deprecated "
                           "in Twisted 16.1. Please use a factory that "
-                          "produces twisted.logger.ILogObserver implementing "
-                          "objects instead."))
+                          "produces twisted.logger.ILogObserver (or the "
+                          "legacy twisted.python.log.ILogObserver) "
+                          "implementing objects instead."))