From: Ira W. Snyder Date: Fri, 31 Oct 2008 03:47:46 +0000 (-0700) Subject: Use exceptions for error handling X-Git-Tag: v2.1.0~4 X-Git-Url: https://www.irasnyder.com/gitweb/?p=rarslave2.git;a=commitdiff_plain;h=3e0a5dd7c7549636eb70c6a641987da66742f1db Use exceptions for error handling The previous error handling strategy was very Linux kernel like, showing influence from what I was working on while I designed this program. Refactor the program to use exceptions, which significantly reduces the amount of code needed for error handling. Best practices are now followed: we only catch exceptions where we need to log a message and rethrow, or for a complete unit of work. Signed-off-by: Ira W. Snyder --- diff --git a/PAR2Set/Base.py b/PAR2Set/Base.py index 533ff4f..a5d3dcb 100644 --- a/PAR2Set/Base.py +++ b/PAR2Set/Base.py @@ -109,25 +109,7 @@ class Base (object): """Verify and Repair a PAR2Set. This is done using the par2repair command by default""" - PAR2_CMD = rsutil.common.config_get_value ('commands', 'par2repair') - - # assemble the command - # par2repair -- PAR2 PAR2_EXTRA [JOIN_FILES] - command = "%s \"%s\" " % (PAR2_CMD, self.p2file) - - for f in self.all_p2files: - if f != self.p2file: - command += "\"%s\" " % os.path.split (f)[1] - - # run the command - ret = rsutil.common.run_command (command, self.dir) - - # check the result - if ret != 0: - logging.critical ('PAR2 Check / Repair failed: %s' % self.p2file) - return -rsutil.common.ECHECK - - return rsutil.common.SUCCESS + rsutil.common.run_command(['par2repair'] + self.all_p2files, self.dir) def find_deleteable_files (self): """Find all files which are deletable by using the regular expression from the @@ -147,32 +129,38 @@ class Base (object): assert os.path.isdir (dir) - done = False - valid_y = ['Y', 'YES'] - valid_n = ['N', 'NO', ''] - + # If we are interactive, decide yes or no if interactive: - while not done: + while True: print 'Do you want to delete the following?:' for f in files: print f - s = raw_input ('Delete [y/N]: ').upper() - if s in valid_y + valid_n: - done = True + s = raw_input('Delete [y/N]: ').upper() + + # If we got a valid no answer, leave now + if s in ['N', 'NO', '']: + return + + # If we got a valid yes answer, delete them + if s in ['Y', 'YES']: + break - if s in valid_n: - return rsutil.common.SUCCESS + # Not a good answer, ask again + print 'Invalid response' + # We got a yes answer (or are non-interactive), delete the files for f in files: - try: - os.remove (os.path.join (dir, f)) - logging.debug ('Deleteing: %s' % os.path.join (dir, f)) - except: - logging.error ('Failed to delete: %s' % os.path.join (dir, f)) - return -rsutil.common.EDELETE - return rsutil.common.SUCCESS + # Get the full filename + fullname = os.path.join(dir, f) + + # Delete the file + try: + os.remove(fullname) + logging.debug('Deleting: %s' % fullname) + except OSError: + logging.error('Failed to delete: %s' % fullname) def runDelete (self): """Run the delete operation and return the result""" @@ -187,78 +175,39 @@ class Base (object): """Run all of the major sections in the class: repair, extraction, and deletion.""" # Repair Stage - ret = self.runVerifyAndRepair () - - if ret != rsutil.common.SUCCESS: - logging.critical ('Repair stage failed for: %s' % self.p2file) - return -rsutil.common.ECHECK + try: + self.runVerifyAndRepair () + except (RuntimeError, OSError): + logging.critical('Repair stage failed for: %s' % self.p2file) + raise - self.update_matches () + self.update_matches() # Extraction Stage - ret = self.runExtract () - - if ret != rsutil.common.SUCCESS: - logging.critical ('Extraction stage failed for: %s' % self.p2file) - return -rsutil.common.EEXTRACT + try: + self.runExtract () + except (RuntimeError, OSError): + logging.critical('Extraction stage failed for: %s' % self.p2file) + raise self.update_matches () # Deletion Stage - ret = self.runDelete () - - if ret != rsutil.common.SUCCESS: - logging.critical ('Deletion stage failed for: %s' % self.p2file) - return -rsutil.common.EDELETE - - logging.info ('Successfully completed: %s' % self.p2file) - return rsutil.common.SUCCESS - - def safe_create_directory (self, dir): - """Safely create a directory, logging the result. - - dir -- the directory to create (None is ignored)""" - - if dir == None: - return rsutil.common.SUCCESS - - if os.path.isdir (dir): - return rsutil.common.SUCCESS - try: - os.makedirs (dir) - logging.info ('Created directory: %s' % dir) - except OSError: - logging.critical ('FAILED to create directory: %s' % dir) - return -rsutil.common.ECREATE + self.runDelete () + except (RuntimeError, OSError): + logging.critical('Deletion stage failed for: %s' % self.p2file) + raise - return rsutil.common.SUCCESS + logging.info ('Successfully completed: %s' % self.p2file) - def runExtract (self, todir=None): + def runExtract (self): """Extract all heads of this set and return the result""" - # Extract to the head's dir if we don't care where to extract - if todir == None: - todir = self.dir - - # Create the directory $todir if it doesn't exist - ret = self.safe_create_directory (todir) - - if ret != rsutil.common.SUCCESS: - return -rsutil.common.EEXTRACT - # Call the extraction function on each head for h in self.find_extraction_heads (): full_head = rsutil.common.full_abspath (os.path.join (self.dir, h)) - ret = self.extraction_function (full_head, todir) - logging.debug ('Extraction Function returned: %d' % ret) - - # Check error code - if ret != rsutil.common.SUCCESS: - logging.critical ('Failed extracting: %s' % h) - return -rsutil.common.EEXTRACT - - return rsutil.common.SUCCESS + self.extraction_function (full_head, self.dir) def find_extraction_heads (self): """Find all extraction heads associated with this set. This must be diff --git a/PAR2Set/ExtractFirstBase.py b/PAR2Set/ExtractFirstBase.py index 6b8a661..7acfc63 100644 --- a/PAR2Set/ExtractFirstBase.py +++ b/PAR2Set/ExtractFirstBase.py @@ -39,30 +39,29 @@ class ExtractFirstBase (PAR2Set.Base.Base): """Run the extraction, repair, and delete stages""" # Extraction Stage - ret = self.runExtract () - - if ret != rsutil.common.SUCCESS: - logging.critical ('Extraction stage failed for: %s' % self.p2file) - return -rsutil.common.EEXTRACT + try: + self.runExtract() + except (RuntimeError, OSError): + logging.critical('Extraction stage failed for: %s' % self.p2file) + raise self.update_matches () # Repair Stage - ret = self.runVerifyAndRepair () - - if ret != rsutil.common.SUCCESS: - logging.critical ('Repair stage failed for: %s' % self.p2file) - return -rsutil.common.ECHECK + try: + self.runVerifyAndRepair() + except (RuntimeError, OSError): + logging.critical('Repair stage failed for: %s' % self.p2file) + raise self.update_matches () # Deletion Stage - ret = self.runDelete () - - if ret != rsutil.common.SUCCESS: - logging.critical ('Deletion stage failed for: %s' % self.p2file) - return -rsutil.common.EDELETE + try: + self.runDelete() + except (RuntimeError, OSError): + logging.critical('Deletion stage failed for: %s' % self.p2file) + raise logging.info ('Successfully completed: %s' % self.p2file) - return rsutil.common.SUCCESS diff --git a/PAR2Set/Join.py b/PAR2Set/Join.py index 61a1c42..1dd680a 100644 --- a/PAR2Set/Join.py +++ b/PAR2Set/Join.py @@ -75,28 +75,7 @@ class Join (PAR2Set.Base.Base): This is done using the par2repair command by default""" - PAR2_CMD = rsutil.common.config_get_value ('commands', 'par2repair') - - # assemble the command - # par2repair -- PAR2 PAR2_EXTRA [JOIN_FILES] - command = "%s \"%s\" " % (PAR2_CMD, self.p2file) - - for f in self.all_p2files: - if f != self.p2file: - command += "\"%s\" " % os.path.split (f)[1] - - for f in self.find_joinfiles (): - command += "\"%s\" " % os.path.split (f)[1] - - # run the command - ret = rsutil.common.run_command (command, self.dir) - - # check the result - if ret != 0: - logging.critical ('PAR2 Check / Repair failed: %s' % self.p2file) - return -rsutil.common.ECHECK - - return rsutil.common.SUCCESS + rsutil.common.run_command(['par2repair'] + self.all_p2files + self.find_joinfiles(), self.dir) def find_deleteable_files (self): """Find all files which are deletable by using the regular expression from the @@ -123,20 +102,5 @@ class Join (PAR2Set.Base.Base): This command ignores the extraction if file and todir+file are the same file. This keeps things like mv working smoothly.""" - NOEXTRACT_CMD = rsutil.common.config_get_value ('commands', 'noextract') - - # Make sure that both files are not the same file. If they are, don't run at all. - # NOTE: os.path.samefile() doesn't exist on win32, so we can't use it. - if rsutil.common.full_abspath (file) == \ - rsutil.common.full_abspath (os.path.join (todir, file)): - return rsutil.common.SUCCESS - - cmd = NOEXTRACT_CMD % (file, todir) - ret = rsutil.common.run_command (cmd) - - # Check error code - if ret != 0: - return -rsutil.common.EEXTRACT - - return rsutil.common.SUCCESS - + # The Join type doesn't need any extraction + pass diff --git a/PAR2Set/NewRAR.py b/PAR2Set/NewRAR.py index caefb29..7c8f25b 100644 --- a/PAR2Set/NewRAR.py +++ b/PAR2Set/NewRAR.py @@ -72,14 +72,5 @@ class NewRAR (PAR2Set.Base.Base): assert os.path.isfile (file) assert os.path.isdir (todir) - RAR_CMD = rsutil.common.config_get_value ('commands', 'unrar') - - cmd = '%s \"%s\"' % (RAR_CMD, file) - ret = rsutil.common.run_command (cmd, todir) - - # Check error code - if ret != 0: - return -rsutil.common.EEXTRACT - - return rsutil.common.SUCCESS + rsutil.common.run_command(['unrar', 'x', '-o+', file], todir) diff --git a/PAR2Set/NoExtract.py b/PAR2Set/NoExtract.py index 51ee235..9b4ea46 100644 --- a/PAR2Set/NoExtract.py +++ b/PAR2Set/NoExtract.py @@ -94,21 +94,20 @@ class NoExtract (PAR2Set.Base.Base): """Run the Repair and Deletion stages, omitting the Extraction stage""" # Repair Stage - ret = self.runVerifyAndRepair () - - if ret != rsutil.common.SUCCESS: - logging.critical ('Repair stage failed for: %s' % self.p2file) - return -rsutil.common.ECHECK + try: + self.runVerifyAndRepair() + except (RuntimeError, OSError): + logging.critical('Repair stage failed for: %s' % self.p2file) + raise self.update_matches () # Deletion Stage - ret = self.runDelete () - - if ret != rsutil.common.SUCCESS: - logging.critical ('Deletion stage failed for: %s' % self.p2file) - return -rsutil.common.EDELETE + try: + self.runDelete() + except (RuntimeError, OSError): + logging.critical('Delete stage failed for: %s' % self.p2file) + raise logging.info ('Successfully completed: %s' % self.p2file) - return rsutil.common.SUCCESS diff --git a/PAR2Set/OldRAR.py b/PAR2Set/OldRAR.py index b872ccf..b139570 100644 --- a/PAR2Set/OldRAR.py +++ b/PAR2Set/OldRAR.py @@ -74,14 +74,5 @@ class OldRAR (PAR2Set.Base.Base): assert os.path.isfile (file) assert os.path.isdir (todir) - RAR_CMD = rsutil.common.config_get_value ('commands', 'unrar') - - cmd = '%s \"%s\"' % (RAR_CMD, file) - ret = rsutil.common.run_command (cmd, todir) - - # Check error code - if ret != 0: - return -rsutil.common.EEXTRACT - - return rsutil.common.SUCCESS + rsutil.common.run_command(['unrar', 'x', '-o+', file], todir) diff --git a/PAR2Set/ZIP.py b/PAR2Set/ZIP.py index 36acb74..39a83ff 100644 --- a/PAR2Set/ZIP.py +++ b/PAR2Set/ZIP.py @@ -69,14 +69,5 @@ class ZIP (PAR2Set.Base.Base): file -- the file to extract todir -- the directory to extract into""" - ZIP_CMD = rsutil.common.config_get_value ('commands', 'unzip') - - cmd = ZIP_CMD % (file, todir) - ret = rsutil.common.run_command (cmd) - - # Check error code - if ret != 0: - return -rsutil.common.EEXTRACT - - return rsutil.common.SUCCESS + rsutil.common.run_command(['unzip', file], todir) diff --git a/RarslaveDetector.py b/RarslaveDetector.py index 73a442b..dfb2768 100644 --- a/RarslaveDetector.py +++ b/RarslaveDetector.py @@ -82,6 +82,7 @@ class RarslaveDetector (object): the class is valid.""" detected = False + success = False for (detector, classname) in self.TYPES: if detector (self.name_matched_files, self.prot_matched_files): @@ -93,15 +94,18 @@ class RarslaveDetector (object): logging.debug ('Detected type: %s' % p2set) # Try to have rarslave do it's thing - ret = p2set.runAll () + try: + # Have rarslave do it's thing + p2set.runAll () - # If something already worked, there is no need to continue, - # since we've already finished! - if ret == rsutil.common.SUCCESS: + # It succeeded, exit the loop early + success = True break - else: - logging.error ('Detected type (%s) failed for: %s' % (p2set, - self.p2file)) + except (OSError, RuntimeError): + logging.error('Detected type (%s) failed for: %s' % (p2set, self.p2file)) + except: + logging.error('Unknown exception occurred') + raise # Make sure we detected at least one valid type if not detected: @@ -109,13 +113,10 @@ class RarslaveDetector (object): logging.debug ('The following information will help in writing a detector:') logging.debug ('name_matches: %s' % self.name_matched_files) logging.debug ('prot_matches: %s' % self.prot_matched_files) - return -rsutil.common.EDETECT # Make sure that something worked - if ret != rsutil.common.SUCCESS: - logging.critical ('All types failed for: %s' % self.p2file) - - return ret + if success == False: + logging.critical('All types failed for: %s' % self.p2file) def main (): pass diff --git a/rarslave.py b/rarslave.py index 7c1b4c8..18bb540 100755 --- a/rarslave.py +++ b/rarslave.py @@ -122,17 +122,28 @@ def generate_all_parsets (dir): def check_required_progs(): """Check if the required programs are installed""" - shell_not_found = 32512 needed = [] - if rsutil.common.run_command ('par2repair --help > /dev/null 2>&1') == shell_not_found: - needed.append ('par2repair') + try: + rsutil.common.run_command(['par2repair', '--help']) + except OSError: + needed.append('par2repair') + except RuntimeError: + pass - if rsutil.common.run_command ('unrar --help > /dev/null 2>&1') == shell_not_found: - needed.append ('unrar') + try: + rsutil.common.run_command(['unrar', '--help']) + except OSError: + needed.append('unrar') + except RuntimeError: + pass - if rsutil.common.run_command ('unzip --help > /dev/null 2>&1') == shell_not_found: - needed.append ('unzip') + try: + rsutil.common.run_command(['unzip', '--help']) + except OSError: + needed.append('unzip') + except RuntimeError: + pass if needed: for n in needed: @@ -169,6 +180,7 @@ def run_options (options): if options.check_progs: check_required_progs () + sys.exit (0) if options.write_def_config: config.write_config (default=True) @@ -264,14 +276,14 @@ def main (): parsets = generate_all_parsets (dir) for (p2dir, p2file) in parsets: detector = RarslaveDetector.RarslaveDetector (p2dir, p2file) - ret = detector.runMatchingTypes () + detector.runMatchingTypes () # Non-recursive else: parsets = generate_all_parsets (options.work_dir) for (p2dir, p2file) in parsets: detector = RarslaveDetector.RarslaveDetector (p2dir, p2file) - ret = detector.runMatchingTypes () + detector.runMatchingTypes () # Print the results if logger.size () > 0: diff --git a/rsutil/common.py b/rsutil/common.py index 66b7d02..370d7d6 100644 --- a/rsutil/common.py +++ b/rsutil/common.py @@ -29,13 +29,12 @@ __license__ = "GNU GPL v2 (or, at your option, any later version)" import os import re import logging +import subprocess +import exceptions import rsutil.globals import rsutil.par2parser -# Global constants -(SUCCESS, ECHECK, EEXTRACT, EDELETE, ECREATE, EDETECT, EPARSE) = range(7) - def find_matches (regex, li, ignorecase=True): if ignorecase: @@ -66,14 +65,22 @@ def run_command (cmd, indir=None): # Runs the specified command-line in the directory given (or, in the current directory # if none is given). It returns the status code given by the application. - pwd = os.getcwd () - - if indir != None: + if indir == None: + indir = os.getcwd() + else: assert os.path.isdir (indir) # MUST be a directory! - os.chdir (indir) - ret = os.system (cmd) - os.chdir (pwd) + print 'RUNNING COMMAND' + print 'Directory: %s' % indir + print 'Command: %s' % cmd[0] + for f in cmd[1:]: + print '-> %s' % f + + ret = subprocess.Popen(cmd, cwd=indir).wait() + + if ret != 0: + raise RuntimeError + return ret def full_abspath (p): @@ -143,11 +150,7 @@ def list_eq (l1, l2): if len(l1) != len(l2): return False - for e in l1: - if e not in l2: - return False - - return True + return set(l1) == set(l2) # Convience functions for the config