Logging

[juggie]

Logging

[rct]

This looks good. The queue is a nice solution to the chicken and egg problem. I've tested with no log file, log file configured, and a log file that can't be created: no such directory and permission denied. All looks good.

In the error cases where the log file can't be created, there is a python exception/stack trace that gives enough information for most people to figure out what's wrong. Minor nit, at the point the exception is thrown, anything that was in the log queue is lost, but it probably doesn't matter.

[juggie]

We can likely write to the log and fall back to console output if that fails. I'll look at that later.

[juggie]

@rct committed a fix for missing logfile to bail out to console.

Edit: Well not missing, but unable to write.

[gerstl]

The fix now fails if the log file is missing: the logfile does not get created when it doesn't exist on launch.

Instead of using os.access(), which returns False if the file doesn't exist, even if it could be created, the proper way to handle the error conditions should probably be to catch the corresponding exception?!

Here is a diff, also includes a patch to lower the logging level if logging to a file, and only show DEBUG messages when logging to the console:

--- a/core/logger.py
+++ b/core/logger.py
@@ -19,9 +19,9 @@ class DispatchingFormatter:
 def start(logfile = None):
     #setup logging handler
     if logfile:
-        if os.access(logfile, os.W_OK):
+        try:
             handler = logging.FileHandler(logfile)
-        else:
+        except IOError:
             handler = logging.StreamHandler()
             error("Unable to open %s for writing" % logfile)
     else:
@@ -36,7 +36,10 @@ def start(logfile = None):

     #setup our handler and log level
     logging.getLogger().addHandler(handler)
-    logging.getLogger().setLevel(logging.DEBUG)
+    if logfile:
+        logging.getLogger().setLevel(logging.INFO)
+    else:
+        logging.getLogger().setLevel(logging.DEBUG)
     start.started = 1
 start.started = 0

[juggie]

Fixed, in the future a pull request would be appreciated.