[Fedora-livecd-list] Patch review request for exception handling

Bruno Wolff III bruno at wolff.to
Sat Jul 24 20:30:16 UTC 2010


I have attached the diff. The commit note is below. I like to get an ack or
two on this patch. Thanks!

    Fix issues due to raises supplying one argument when two are expected.
    
    OSError and IOError normally have 2 or 3 arguments. However some routines
    raise these with only one argument. This patch changes the handling to
    use just the last argument provided (which would be the error message) or
    the empty string if there isn't one.
    
    I didn't print the list of arguments, since for these two exceptions the
    first argument is an error number, that wouldn't look good in the error
    message.
    
    I didn't message with any of non-builtin exceptions, as I am assuming that
    in those cases a mismatch is an error that should get fixed. Most of those
    cases use the whole argument list as the error message in any case.
    
    This resolves bug 551932.
-------------- next part --------------
diff --git a/imgcreate/creator.py b/imgcreate/creator.py
index 4deb49e..9fddca1 100644
--- a/imgcreate/creator.py
+++ b/imgcreate/creator.py
@@ -385,9 +385,9 @@ class ImageCreator(object):
         try:
             self.__builddir = tempfile.mkdtemp(dir =  os.path.abspath(self.tmpdir),
                                                prefix = "imgcreate-")
-        except OSError, (err, msg):
+        except OSError, e:
             raise CreatorError("Failed create build directory in %s: %s" %
-                               (self.tmpdir, msg))
+                               (self.tmpdir, ''.join(e.args[-1:])))
 
     def __sanity_check(self):
         """Ensure that the config we've been given is sane."""
@@ -705,9 +705,10 @@ class ImageCreator(object):
             try:
                 subprocess.check_call([s.interp, script],
                                       preexec_fn = preexec, env = env)
-            except OSError, (err, msg):
+            except OSError, e:
                 raise CreatorError("Failed to execute %%post script "
-                                   "with '%s' : %s" % (s.interp, msg))
+                                   "with '%s' : %s" %
+                                   (s.interp, ''.join(e.args[-1:])))
             except subprocess.CalledProcessError, err:
                 if s.errorOnFail:
                     raise CreatorError("%%post script failed with code %d "
diff --git a/imgcreate/debug.py b/imgcreate/debug.py
index c45047d..e3045ce 100644
--- a/imgcreate/debug.py
+++ b/imgcreate/debug.py
@@ -30,9 +30,9 @@ def handle_logging(option, opt, val, parser, logger, level):
 def handle_logfile(option, opt, val, parser, logger, stream):
     try:
         logfile = logging.FileHandler(val,"a")
-    except IOError, (err, msg):
+    except IOError, e:
         raise optparse.OptionValueError("Cannot open file '%s' : %s" %
-                                        (val, msg))
+                                        (val, ''.join(e.args[-1:])))
 
 
     logger.removeHandler(stream)
diff --git a/imgcreate/fs.py b/imgcreate/fs.py
index 16445a5..33d7f68 100644
--- a/imgcreate/fs.py
+++ b/imgcreate/fs.py
@@ -36,8 +36,10 @@ def makedirs(dirname):
     """
     try:
         os.makedirs(dirname)
-    except OSError, (err, msg):
-        if err != errno.EEXIST:
+    except OSError, e:
+        if len(e.args) < 2:
+            raise
+        if e.args[0] != errno.EEXIST:
             raise
 
 def mksquashfs(in_img, out_img, compress_type):
diff --git a/imgcreate/kickstart.py b/imgcreate/kickstart.py
index 328043c..3a83bf1 100644
--- a/imgcreate/kickstart.py
+++ b/imgcreate/kickstart.py
@@ -53,9 +53,9 @@ def read_kickstart(path):
     try:
         ksfile = urlgrabber.urlgrab(path)
         ks.readKickstart(ksfile)
-    except IOError, (err, msg):
+    except IOError, e:
         raise errors.KickstartError("Failed to read kickstart file "
-                                    "'%s' : %s" % (path, msg))
+                                    "'%s' : %s" % (path, ''.join(e.args[-1:])))
     except kserrors.KickstartError, e:
         raise errors.KickstartError("Failed to parse kickstart file "
                                     "'%s' : %s" % (path, e))
@@ -153,8 +153,8 @@ class TimezoneConfig(KickstartConfig):
         try:
             shutil.copyfile(self.path("/usr/share/zoneinfo/%s" %(tz,)),
                             self.path("/etc/localtime"))
-        except OSError, (errno, msg):
-            log.error("Error copying timezone: %s" %(msg,))
+        except OSError, e:
+            log.error("Error copying timezone: %s" %(''.join(e.args[-1:]),))
 
 
 class AuthConfig(KickstartConfig):


More information about the livecd mailing list