[Fedora-livecd-list] [PATCH] Make use of python logging API for debug messages

Mark McLoughlin markmc at redhat.com
Wed Apr 23 08:35:16 UTC 2008


Hey,
	I found myself replying to this elsewhere ... replying again in the
appropriate place :-)

  - Importing sub-modules isn't supposed to be part of the public API. 
    See __all__ in init.py for what is supposed to be public. You'll 
    want to add add_logging_options() and setup_logging() there

    One thing we could probably never do after adding this API is have 
    add_logging_options() add more options, otherwise we'd run the risk 
    of breaking some existing options in program using it

  - I'd pass the full options object to setup_logging() rather than the 
    individual members - that way it can be opaque what members 
    add_logging_options() add

  - Actually, rather than have setup_logging(), we should have each of
    the options handled by a callback ...

  - add_logging_options() needs a docstring documenting which options 
    it adds, that no more will be added in future and that it affects
    the root logger

  - Should cleanly handle exceptions when the logfile can't be opened

In the end, I just tried out the idea of using callbacks myself ...
updated patch attached.

Cheers,
Mark.

From: Daniel P. Berrange <berrange at redhat.com>
Subject: [PATCH] Make use of python logging API for debug messages

Add a new setup_logging() convenience API for adding
command line options related to debugging.

Also, make everything use the root logger for logging
messages.

Signed-off-by: Mark McLoughlin <markmc at redhat.com>
---
 imgcreate/__init__.py |    4 ++-
 imgcreate/creator.py  |    5 ++-
 imgcreate/debug.py    |   96 +++++++++++++++++++++++++++++++++++++++++++++++++
 imgcreate/live.py     |    4 +-
 imgcreate/yuminst.py  |    3 +-
 tools/image-creator   |    7 +++-
 tools/livecd-creator  |    8 +++-
 7 files changed, 117 insertions(+), 10 deletions(-)
 create mode 100644 imgcreate/debug.py

diff --git a/imgcreate/__init__.py b/imgcreate/__init__.py
index e535014..bcfb40e 100644
--- a/imgcreate/__init__.py
+++ b/imgcreate/__init__.py
@@ -21,6 +21,7 @@ from imgcreate.creator import *
 from imgcreate.yuminst import *
 from imgcreate.kickstart import *
 from imgcreate.fs import *
+from imgcreate.debug import *
 
 """A set of classes for building Fedora system images.
 
@@ -62,5 +63,6 @@ __all__ = (
     'LoopImageCreator',
     'FSLABEL_MAXLEN',
     'read_kickstart',
-    'construct_name'
+    'construct_name',
+    'setup_logging',
 )
diff --git a/imgcreate/creator.py b/imgcreate/creator.py
index 1028e32..febb847 100644
--- a/imgcreate/creator.py
+++ b/imgcreate/creator.py
@@ -22,6 +22,7 @@ import stat
 import sys
 import tempfile
 import shutil
+import logging
 
 import yum
 import rpm
@@ -522,7 +523,7 @@ class ImageCreator(object):
                                        (pkg, e))
 
         for pkg in skipped_pkgs:
-            print >> sys.stderr, "Skipping missing package '%s'" % (pkg,)
+            logging.info("Skipping missing package '%s'" % (pkg,))
 
     def __select_groups(self, ayum):
         skipped_groups = []
@@ -537,7 +538,7 @@ class ImageCreator(object):
                     skipped_groups.append(group)
 
         for group in skipped_groups:
-            print >> sys.stderr, "Skipping missing group '%s'" % (group.name,)
+            logging.info("Skipping missing group '%s'" % (group.name,))
 
     def __deselect_packages(self, ayum):
         for pkg in kickstart.get_excluded(self.ks,
diff --git a/imgcreate/debug.py b/imgcreate/debug.py
new file mode 100644
index 0000000..b8040b1
--- /dev/null
+++ b/imgcreate/debug.py
@@ -0,0 +1,96 @@
+#
+# debug.py: Helper routines for debugging
+#
+# Copyright 2008, Red Hat  Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Library General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+#
+
+import logging
+import logging.handlers
+import optparse
+import sys
+
+DATE_FORMAT   = "%a, %d %b %Y %H:%M:%S"
+STREAM_FORMAT = "%(levelname)-6s %(message)s"
+FILE_FORMAT   = "[%(asctime)s %(process)d] %(levelname)s (%(module)s:%(lineno)d) %(message)s"
+
+LOGFILE_MAX_BYTES = 1024 * 1024
+LOGFILE_N_BACKUPS = 5
+
+def handle_logging(option, opt, val, parser, logger, level):
+    if level < logger.level:
+        logger.setLevel(level)
+
+def handle_logfile(option, opt, val, parser, logger, stream):
+    try:
+        logfile = logging.handlers.RotatingFileHandler(val, "a",
+                                                       LOGFILE_MAX_BYTES,
+                                                       LOGFILE_N_BACKUPS)
+    except IOError, (err, msg):
+        raise optparse.OptionValueError("Cannot open file '%s' : %s" %
+                                        (val, msg))
+
+    logfile.setFormatter(logging.Formatter(FILE_FORMAT, DATE_FORMAT))
+
+    logger.removeHandler(stream)
+    logger.addHandler(logfile)
+
+def setup_logging(parser = None):
+    """Set up the root logger and add logging options.
+
+    Set up the root logger so only warning/error messages are logged to stderr
+    by default.
+
+    Also, optionally, add --debug, --verbose and --logfile command line options
+    to the supplied option parser, allowing the root logger configuration to be
+    modified by the user.
+
+    Note, to avoid possible namespace clashes, setup_logging() will only ever
+    add these three options. No new options will be added in the future.
+
+    parser -- an optparse.OptionParser instance, or None
+
+    """
+    logger = logging.getLogger()
+
+    logger.setLevel(logging.WARN)
+
+    stream = logging.StreamHandler(sys.stderr)
+    stream.setFormatter(logging.Formatter(STREAM_FORMAT, DATE_FORMAT))
+
+    logger.addHandler(stream)
+
+    if parser is None:
+        return
+
+    group = optparse.OptionGroup(parser, "Debugging options",
+                                 "These options control the output of logging information during image creation")
+
+    group.add_option("-d", "--debug",
+                     action = "callback", callback = handle_logging,
+                     callback_args = (logger, logging.DEBUG),
+                     help = "Output debugging information")
+
+    group.add_option("-v", "--verbose",
+                     action = "callback", callback = handle_logging,
+                     callback_args = (logger, logging.INFO),
+                     help = "Output verbose progress information")
+
+    group.add_option("", "--logfile", type="string",
+                     action = "callback", callback = handle_logfile,
+                     callback_args = (logger, stream),
+                     help = "Save debug information to FILE", metavar = "FILE")
+
+    parser.add_option_group(group)
diff --git a/imgcreate/live.py b/imgcreate/live.py
index c6267f7..130f7fa 100644
--- a/imgcreate/live.py
+++ b/imgcreate/live.py
@@ -21,6 +21,7 @@ import os.path
 import glob
 import shutil
 import subprocess
+import logging
 
 from imgcreate.errors import *
 from imgcreate.fs import *
@@ -252,8 +253,7 @@ class LiveImageCreatorBase(LoopImageCreator):
         elif os.path.exists("/usr/lib/anaconda-runtime/implantisomd5"):
             implantisomd5 = "/usr/lib/anaconda-runtime/implantisomd5"
         else:
-            print >> sys.stderr, \
-                  "isomd5sum not installed; not setting up mediacheck"
+            logging.warn("isomd5sum not installed; not setting up mediacheck")
             
         subprocess.call([implantisomd5, iso])
 
diff --git a/imgcreate/yuminst.py b/imgcreate/yuminst.py
index 80fc514..aebb822 100644
--- a/imgcreate/yuminst.py
+++ b/imgcreate/yuminst.py
@@ -18,6 +18,7 @@
 
 import os
 import sys
+import logging
 
 import yum
 import rpmUtils
@@ -103,7 +104,7 @@ class LiveCDYum(yum.YumBase):
                         pkgs.remove(x)
                         self.tsInfo.conditionals[req] = pkgs
         else:
-            print >> sys.stderr, "No such package %s to remove" %(pkg,)
+            logging.warn("No such package %s to remove" %(pkg,))
 
     def selectGroup(self, grp, include = pykickstart.parser.GROUP_DEFAULT):
         yum.YumBase.selectGroup(self, grp)
diff --git a/tools/image-creator b/tools/image-creator
index aca9228..6f2604c 100755
--- a/tools/image-creator
+++ b/tools/image-creator
@@ -21,6 +21,7 @@ import os
 import sys
 import shutil
 import optparse
+import logging
 
 import imgcreate
 
@@ -30,6 +31,8 @@ def parse_options(args):
     parser.add_option("-n", "--name", type="string", dest="name",
                       help="Image name and filesystem label")
 
+    imgcreate.setup_logging(parser)
+
     (options, args) = parser.parse_args()
 
     if len(args) != 1:
@@ -48,7 +51,7 @@ def main():
     try:
         ks = imgcreate.read_kickstart(kscfg)
     except imgcreate.CreatorError, e:
-        print >> sys.stderr, "Error loading kickstart file '%s' : %s" % (kscfg, e)
+        logging.error("Unable to load kickstart file '%s' : %s" % (kscfg, e))
         return 1
 
     if options.name:
@@ -61,7 +64,7 @@ def main():
     try:
         creator.create()
     except imgcreate.CreatorError, e:
-        print >> sys.stderr, "Error creating image : %s" % e
+        logging.error("Unable to create image : %s" % e)
         return 1
     finally:
         creator.cleanup()
diff --git a/tools/livecd-creator b/tools/livecd-creator
index 7c08323..dddfddd 100755
--- a/tools/livecd-creator
+++ b/tools/livecd-creator
@@ -22,8 +22,10 @@ import os.path
 import sys
 import time
 import optparse
+import logging
 
 import imgcreate
+import imgcreate.debug
 
 class Usage(Exception):
     def __init__(self, msg = None, no_error = False):
@@ -53,6 +55,8 @@ def parse_options(args):
                       help="Cache directory to use (default: private cache")
     parser.add_option_group(sysopt)
 
+    imgcreate.setup_logging(parser)
+
     # debug options not recommended for "production" images
     # Start a shell in the chroot for post-configuration.
     parser.add_option("-l", "--shell", action="store_true", dest="give_shell",
@@ -101,7 +105,7 @@ def main():
                                         "livecd-",
                                         maxlen = imgcreate.FSLABEL_MAXLEN)
 
-        print "Using label '%s' and name '%s'" % (fs_label, name)
+        logging.info("Using label '%s' and name '%s'" % (fs_label, name))
 
     ks = imgcreate.read_kickstart(options.kscfg)
 
@@ -121,7 +125,7 @@ def main():
         creator.unmount()
         creator.package()
     except imgcreate.CreatorError, e:
-        print >> sys.stderr, "Error creating Live CD : %s" % e
+        logging.error("Error creating Live CD : %s" % e)
         return 1
     finally:
         creator.cleanup()
-- 
1.5.4.1






More information about the Fedora-livecd-list mailing list