From 14b8f427a2dab755a8791e6805efd2aed591c379 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 9 Dec 2014 00:15:33 -0800 Subject: [PATCH] Code quality/lint warnings --- letsencrypt/client/apache_configurator.py | 6 ++-- letsencrypt/client/augeas_configurator.py | 36 +++++++++---------- .../client/tests/apache_configurator_test.py | 18 ++++++---- 3 files changed, 31 insertions(+), 29 deletions(-) diff --git a/letsencrypt/client/apache_configurator.py b/letsencrypt/client/apache_configurator.py index 4d686a345..8a50eb7bf 100644 --- a/letsencrypt/client/apache_configurator.py +++ b/letsencrypt/client/apache_configurator.py @@ -7,7 +7,6 @@ import shutil import socket import subprocess import sys -import time from Crypto import Random @@ -585,12 +584,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ if_mods = self.aug.match(("%s/IfModule/*[self::arg='%s']" % - (aug_conf_path, mod))) + (aug_conf_path, mod))) if len(if_mods) == 0: self.aug.set("%s/IfModule[last() + 1]" % aug_conf_path, "") self.aug.set("%s/IfModule[last()]/arg" % aug_conf_path, mod) if_mods = self.aug.match(("%s/IfModule/*[self::arg='%s']" % - (aug_conf_path, mod))) + (aug_conf_path, mod))) # Strip off "arg" at end of first ifmod path return if_mods[0][:len(if_mods[0]) - 3] @@ -1463,6 +1462,7 @@ LogLevel warn \n\ # SHOWING A NICE ERROR MESSAGE ABOUT THE PROBLEM # Check to make sure options-ssl.conf is installed + # pylint: disable=no-member if not os.path.isfile(CONFIG.OPTIONS_SSL_CONF): dist_conf = pkg_resources.resource_filename( __name__, os.path.basename(CONFIG.OPTIONS_SSL_CONF)) diff --git a/letsencrypt/client/augeas_configurator.py b/letsencrypt/client/augeas_configurator.py index 723d163ea..9aa4ac75b 100644 --- a/letsencrypt/client/augeas_configurator.py +++ b/letsencrypt/client/augeas_configurator.py @@ -87,7 +87,7 @@ class AugeasConfigurator(configurator.Configurator): try: # This is a noop save self.aug.save() - except: + except (RuntimeError, IOError): # Check for the root of save problems new_errs = self.aug.match("/augeas//error") # logger.error("During Save - " + mod_conf) @@ -167,7 +167,7 @@ class AugeasConfigurator(configurator.Configurator): """ try: rollback = int(rollback) - except: + except ValueError: logger.error("Rollback argument must be a positive integer") # Sanity check input if rollback < 1: @@ -179,7 +179,7 @@ class AugeasConfigurator(configurator.Configurator): if len(backups) < rollback: logger.error(("Unable to rollback %d checkpoints, only " - "%d exist") % (rollback, len(backups))) + "%d exist") % (rollback, len(backups))) while rollback > 0 and backups: cp_dir = self.direc["backup"] + backups.pop() @@ -212,31 +212,29 @@ class AugeasConfigurator(configurator.Configurator): try: for bkup in backups: float(bkup) - except: + except ValueError: assert False, "Invalid files in %s" % self.direc["backup"] for bkup in backups: print time.ctime(float(bkup)) - with open(os.path.join(self.direc["backup"] + bkup, - "CHANGES_SINCE")) as changes_fd: + cur_dir = self.direc["backup"] + bkup + with open(os.path.join(cur_dir, "CHANGES_SINCE")) as changes_fd: print changes_fd.read() print "Affected files:" - with open( - self.direc["backup"] + bkup + "/FILEPATHS") as paths_fd: + with open(os.path.join(cur_dir, "FILEPATHS")) as paths_fd: filepaths = paths_fd.read().splitlines() for path in filepaths: print " %s" % path try: - with open( - self.direc["backup"] + bkup + "/NEW_FILES") as new_fd: + with open(os.path.join(cur_dir, "NEW_FILES")) as new_fd: print "New Configuration Files:" filepaths = new_fd.read().splitlines() for path in filepaths: print " %s" % path - except: - pass + except (IOError, OSError) as exc: + print exc print "" def add_to_checkpoint(self, cp_dir, save_files): @@ -294,8 +292,8 @@ class AugeasConfigurator(configurator.Configurator): shutil.copy2(os.path.join( cp_dir, os.path.basename(path) + '_' + str(idx)), - path) - except: + path) + except (IOError, OSError): # This file is required in all checkpoints. logger.error("Unable to recover files from %s" % cp_dir) return 1 @@ -305,7 +303,7 @@ class AugeasConfigurator(configurator.Configurator): try: shutil.rmtree(cp_dir) - except: + except OSError: logger.error("Unable to remove directory: %s" % cp_dir) return -1 @@ -355,7 +353,7 @@ class AugeasConfigurator(configurator.Configurator): with open(os.path.join(cp_dir, "NEW_FILES"), 'a') as new_fd: for file_path in files: new_fd.write("%s\n" % file_path) - except: + except (IOError, OSError): logger.error("ERROR: Unable to register file creation") def recovery_routine(self): @@ -409,7 +407,7 @@ class AugeasConfigurator(configurator.Configurator): "File: %s - Could not be found to be deleted\n" "Program was probably shut down unexpectedly, " "in which case this is not a problem") % path) - except IOError: + except (IOError, OSError): logger.fatal( "Unable to remove filepaths contained within %s" % file_list) sys.exit(41) @@ -442,12 +440,12 @@ class AugeasConfigurator(configurator.Configurator): shutil.move(changes_since_tmp_path, changes_since_path) - except: + except (IOError, OSError): logger.error("Unable to finalize checkpoint - adding title") return False try: os.rename(cp_dir, final_dir) - except: + except OSError: logger.error("Unable to finalize checkpoint, %s -> %s" % (cp_dir, final_dir)) return False diff --git a/letsencrypt/client/tests/apache_configurator_test.py b/letsencrypt/client/tests/apache_configurator_test.py index 53fbfad9c..f52d236f5 100644 --- a/letsencrypt/client/tests/apache_configurator_test.py +++ b/letsencrypt/client/tests/apache_configurator_test.py @@ -12,7 +12,6 @@ import sys import unittest from letsencrypt.client import apache_configurator -from letsencrypt.client import CONFIG from letsencrypt.client import display from letsencrypt.client import logger @@ -21,7 +20,7 @@ TESTING_DIR = os.path.dirname(os.path.realpath(__file__)) UBUNTU_CONFIGS = os.path.join(TESTING_DIR, "debian_apache_2_4/") TEMP_DIR = os.path.join(TESTING_DIR, "temp") - +# pylint: disable=invalid-name def setUpModule(): """Run once before all unittests.""" logger.setLogger(logger.FileLogger(sys.stdout)) @@ -34,7 +33,7 @@ def setUpModule(): shutil.copytree(UBUNTU_CONFIGS, TEMP_DIR, symlinks=True) - +# pylint: disable=invalid-name def tearDownModule(): """Run once after all unittests.""" shutil.rmtree(TEMP_DIR) @@ -45,11 +44,11 @@ class TwoVhost80(unittest.TestCase): @mock.patch("letsencrypt.client.apache_configurator." "subprocess.Popen") - def setUp(self, mock_Popen): + def setUp(self, mock_popen): # pylint: disable=invalid-name """Run before each and every test.""" # This just states that the ssl module is already loaded - mock_Popen.return_value = my_Popen() + mock_popen.return_value = MyPopen() # Final slash is currently important self.config_path = os.path.join(TEMP_DIR, "two_vhost_80/apache2/") @@ -91,6 +90,7 @@ class TwoVhost80(unittest.TestCase): self.vh_truth[2].add_name("ip-172-30-0-17") self.vh_truth[3].add_name("letsencrypt.demo") + # pylint: disable=protected-access def test_parse_file(self): """test parse_file. @@ -201,6 +201,7 @@ class TwoVhost80(unittest.TestCase): self.assertTrue(self.config.find_directive( "NameVirtualHost", re.escape("*:443"))) + # pylint: disable=protected-access def test_add_dir_to_ifmodssl(self): """test _add_dir_to_ifmodssl. @@ -258,8 +259,11 @@ def debug_file(filepath): # I am sure there is a cleaner way to do this... but it works -class my_Popen(object): - def communicate(self): +# pylint: disable=too-few-public-methods +class MyPopen(object): + """Made for mock popen object.""" + def communicate(self): #pylint: disable=no-self-use + """Simply return that ssl_module is in output.""" return "ssl_module", "" if __name__ == '__main__':