From 3048287985946b27cb7a696172a6b1e1e5d65454 Mon Sep 17 00:00:00 2001 From: Javier Pena Date: Thu, 24 Sep 2020 15:34:10 +0200 Subject: [PATCH] Update to a newer hacking version, fix pep8 errors We had a very old hacking version, so we should update it to the same version as other projects are using. The update also required additional changes to the Python code to fix new errors and warnings. Change-Id: Ic511513057581841fe2230b69a2a413ec0981f15 --- packstack/installer/output_messages.py | 2 +- packstack/installer/processors.py | 1 + packstack/installer/run_setup.py | 28 ++++++++++---------- packstack/installer/setup_controller.py | 2 +- packstack/modules/ospluginutils.py | 4 ++- packstack/modules/puppet.py | 34 ++++++++++++------------- packstack/plugins/cinder_250.py | 2 +- packstack/plugins/neutron_350.py | 2 +- packstack/plugins/prescript_000.py | 2 +- packstack/plugins/puppet_950.py | 2 +- packstack/plugins/ssl_001.py | 2 +- packstack/plugins/swift_600.py | 2 +- test-requirements.txt | 4 +-- tests/installer/test_run_setup.py | 2 +- tests/installer/test_validators.py | 4 +-- tox.ini | 2 +- 16 files changed, 49 insertions(+), 46 deletions(-) diff --git a/packstack/installer/output_messages.py b/packstack/installer/output_messages.py index cd9bc0a86..36d28f885 100644 --- a/packstack/installer/output_messages.py +++ b/packstack/installer/output_messages.py @@ -23,7 +23,7 @@ you can relocate %s position in the text as long as the context is kept. DONT CHANGE any of the params names (in UPPER-CASE) they are used in the engine-setup.py -''' +''' # noqa: W605 from packstack.installer import basedefs diff --git a/packstack/installer/processors.py b/packstack/installer/processors.py index 3c3054711..6e0b312fb 100644 --- a/packstack/installer/processors.py +++ b/packstack/installer/processors.py @@ -154,6 +154,7 @@ def process_bool(param, param_name, config=None): elif param.lower() in ('n', 'no', 'false'): return False + # Define silent processors for proc_func in (process_bool, process_add_quotes_around_values): proc_func.silent = True diff --git a/packstack/installer/run_setup.py b/packstack/installer/run_setup.py index 4ffb17224..6a15b67b8 100644 --- a/packstack/installer/run_setup.py +++ b/packstack/installer/run_setup.py @@ -73,7 +73,7 @@ def initLogging(debug): logging.root.handlers = [] logging.root.addHandler(hdlr) logging.root.setLevel(level) - except: + except Exception: logging.error(traceback.format_exc()) raise Exception(output_messages.ERR_EXP_FAILED_INIT_LOGGER) @@ -150,7 +150,7 @@ def _getInputFromUser(param): # add the new line so messages wont be displayed in the same line as the question print("") raise - except: + except Exception: logging.error(traceback.format_exc()) raise Exception(output_messages.ERR_EXP_READ_INPUT_PARAM % (param.CONF_NAME)) @@ -256,7 +256,7 @@ def mask(input): output.remove(org) output.insert(orgIndex, item) if isinstance(input, six.string_types): - output = utils.mask_string(input, masked_value_set) + output = utils.mask_string(input, masked_value_set) return output @@ -531,7 +531,7 @@ def _handleInteractiveParams(): except Exception: logging.error(traceback.format_exc()) raise - except: + except Exception: logging.error(traceback.format_exc()) raise Exception(output_messages.ERR_EXP_HANDLE_PARAMS) @@ -570,16 +570,16 @@ def _displaySummary(): for param in group.parameters.itervalues(): if not param.USE_DEFAULT and param.CONF_NAME in controller.CONF: cmdOption = param.CMD_OPTION - l = 30 - len(cmdOption) + length = 30 - len(cmdOption) maskParam = param.MASK_INPUT # Only call mask on a value if the param has MASK_INPUT set to True if maskParam: logging.info("%s: %s" % (cmdOption, mask(controller.CONF[param.CONF_NAME]))) - print("%s:" % (cmdOption) + " " * l + mask(controller.CONF[param.CONF_NAME])) + print("%s:" % (cmdOption) + " " * length + mask(controller.CONF[param.CONF_NAME])) else: # Otherwise, log & display it as it is logging.info("%s: %s" % (cmdOption, str(controller.CONF[param.CONF_NAME]))) - print("%s:" % (cmdOption) + " " * l + str(controller.CONF[param.CONF_NAME])) + print("%s:" % (cmdOption) + " " * length + str(controller.CONF[param.CONF_NAME])) logging.info("*** User input summary ***") answer = _askYesNo(output_messages.INFO_USE_PARAMS) if not answer: @@ -900,9 +900,9 @@ def plugin_compare(x, y): Used to sort the plugin file list according to the number at the end of the plugin module """ - x_match = re.search(".+\_(\d\d\d)", x) + x_match = re.search(r'.+\_(\d\d\d)', x) x_cmp = x_match.group(1) - y_match = re.search(".+\_(\d\d\d)", y) + y_match = re.search(r'.+\_(\d\d\d)', y) y_cmp = y_match.group(1) return int(x_cmp) - int(y_cmp) @@ -918,7 +918,7 @@ def loadPlugins(): fileList = sorted(fileList, key=cmp_to_key(plugin_compare)) for item in fileList: # Looking for files that end with ###.py, example: a_plugin_100.py - match = re.search("^(.+\_\d\d\d)\.py$", item) + match = re.search(r'^(.+\_\d\d\d)\.py$', item) if match: try: moduleToLoad = match.group(1) @@ -928,7 +928,7 @@ def loadPlugins(): globals()[moduleToLoad] = moduleobj checkPlugin(moduleobj) controller.addPlugin(moduleobj) - except: + except Exception: logging.error("Failed to load plugin from file %s", item) logging.error(traceback.format_exc()) raise Exception("Failed to load plugin from file %s" % item) @@ -1046,9 +1046,9 @@ def main(): # If using an answer file, setting a default password # does not really make sense if getattr(options, 'default_password', None): - msg = ('Please do not set --default-password ' - 'when specifying an answer file.') - raise FlagValidationError(msg) + msg = ('Please do not set --default-password ' + 'when specifying an answer file.') + raise FlagValidationError(msg) confFile = os.path.expanduser(options.answer_file) if not os.path.exists(confFile): raise Exception(output_messages.ERR_NO_ANSWER_FILE % confFile) diff --git a/packstack/installer/setup_controller.py b/packstack/installer/setup_controller.py index 6fcbbb0fa..6fd895ead 100644 --- a/packstack/installer/setup_controller.py +++ b/packstack/installer/setup_controller.py @@ -44,7 +44,7 @@ class Controller(object): instance from a class which inherit Controller. did not use isinstance because inheritence makes it behave erratically. """ - if self != type(self.__single): # flake8: noqa + if self != type(self.__single): # noqa: E721 self.__single = object.__new__(self, *args, **kwargs) return self.__single diff --git a/packstack/modules/ospluginutils.py b/packstack/modules/ospluginutils.py index 27072d649..b1218679c 100644 --- a/packstack/modules/ospluginutils.py +++ b/packstack/modules/ospluginutils.py @@ -66,6 +66,8 @@ class ManifestFiles(object): fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) with os.fdopen(fd, 'w') as fp: fp.write(data) + + manifestfiles = ManifestFiles() @@ -113,7 +115,7 @@ def generate_ssl_cert(config, host, service, ssl_key_file, ssl_cert_file): subject.C = config['CONFIG_SSL_CERT_SUBJECT_C'] subject.ST = config['CONFIG_SSL_CERT_SUBJECT_ST'] subject.L = config['CONFIG_SSL_CERT_SUBJECT_L'] - subject.O = config['CONFIG_SSL_CERT_SUBJECT_O'] + subject.O = config['CONFIG_SSL_CERT_SUBJECT_O'] # noqa: E741 subject.OU = config['CONFIG_SSL_CERT_SUBJECT_OU'] cn = "%s/%s" % (service, fqdn) # if subject.CN is more than 64 chars long, cert creation will fail diff --git a/packstack/modules/puppet.py b/packstack/modules/puppet.py index bbc76137d..22b4aacbd 100644 --- a/packstack/modules/puppet.py +++ b/packstack/modules/puppet.py @@ -22,15 +22,15 @@ from packstack.installer.exceptions import PuppetError # TODO: Fill logger name when logging system will be refactored logger = logging.getLogger() -re_color = re.compile('\x1b.*?\d\dm') +re_color = re.compile(r'\x1b.*?\d\dm') re_error = re.compile( - 'err:|Syntax error at|^Duplicate definition:|^Invalid tag|' - '^No matching value for selector param|^Parameter name failed:|Error:|' - '^Invalid parameter|^Duplicate declaration:|^Could not find resource|' - '^Could not parse for|^/usr/bin/puppet:\d+: .+|.+\(LoadError\)|' - '^Could not autoload|' - '^\/usr\/bin\/env\: jruby\: No such file or directory|' - 'failed to execute puppet' + r'err:|Syntax error at|^Duplicate definition:|^Invalid tag|' + r'^No matching value for selector param|^Parameter name failed:|Error:|' + r'^Invalid parameter|^Duplicate declaration:|^Could not find resource|' + r'^Could not parse for|^/usr/bin/puppet:\d+: .+|.+\(LoadError\)|' + r'^Could not autoload|' + r'^\/usr\/bin\/env\: jruby\: No such file or directory|' + r'failed to execute puppet' ) re_ignore = re.compile( # Puppet preloads a provider using the mysql command before it is installed @@ -45,23 +45,23 @@ re_ignore = re.compile( # https://tickets.puppetlabs.com/browse/FACT-697 'NetworkManager is not running' ) -re_notice = re.compile(r"notice: .*Notify\[packstack_info\]" - "\/message: defined \'message\' as " - "\'(?P.*)\'") +re_notice = re.compile(r'notice: .*Notify\[packstack_info\]' + r'\/message: defined \'message\' as ' + r'\'(?P.*)\'') surrogates = [ # Value in /etc/sysctl.conf cannot be changed - ('Sysctl::Value\[.*\]\/Sysctl\[(?P.*)\].*Field \'val\' is required', + (r'Sysctl::Value\[.*\]\/Sysctl\[(?P.*)\].*Field \'val\' is required', 'Cannot change value of %(arg1)s in /etc/sysctl.conf'), # Package is not found in yum repos - ('Package\[.*\]\/ensure.*yum.*install (?P.*)\'.*Nothing to do', + (r'Package\[.*\]\/ensure.*yum.*install (?P.*)\'.*Nothing to do', 'Package %(arg1)s has not been found in enabled Yum repos.'), - ('Execution of \'.*yum.*install (?P.*)\'.*Nothing to do', + (r'Execution of \'.*yum.*install (?P.*)\'.*Nothing to do', 'Package %(arg1)s has not been found in enabled Yum repos.'), # Packstack does not cooperate with jruby - ('jruby', 'Your Puppet installation uses jruby instead of ruby. Package ' - 'jruby does not cooperate with Packstack well. You will have to ' - 'fix this manually.'), + (r'jruby', 'Your Puppet installation uses jruby instead of ruby. Package ' + 'jruby does not cooperate with Packstack well. You will have to ' + 'fix this manually.'), ] diff --git a/packstack/plugins/cinder_250.py b/packstack/plugins/cinder_250.py index 86306fd8b..472b9c091 100644 --- a/packstack/plugins/cinder_250.py +++ b/packstack/plugins/cinder_250.py @@ -689,7 +689,7 @@ def check_cinder_vg(config, messages): if not have_cinders_volume: raise exceptions.MissingRequirements("The cinder server should " "contain a volume group") - match = re.match('^(?P\d+)G$', + match = re.match(r'^(?P\d+)G$', config['CONFIG_CINDER_VOLUMES_SIZE'].strip()) if not match: msg = 'Invalid Cinder volumes VG size.' diff --git a/packstack/plugins/neutron_350.py b/packstack/plugins/neutron_350.py index 97cb623f4..765ca0ecd 100644 --- a/packstack/plugins/neutron_350.py +++ b/packstack/plugins/neutron_350.py @@ -808,7 +808,7 @@ def create_manifests(config, messages): else: iface = config['CONFIG_NEUTRON_OVS_TUNNEL_IF'] ifip = ("ipaddress_%s" % iface) - ifip = re.sub('[\.\-\:]', '_', ifip) + ifip = re.sub(r'[\.\-\:]', '_', ifip) try: src_host = config['HOST_DETAILS'][n_host][ifip] except KeyError: diff --git a/packstack/plugins/prescript_000.py b/packstack/plugins/prescript_000.py index 5179509d5..aebed36c7 100755 --- a/packstack/plugins/prescript_000.py +++ b/packstack/plugins/prescript_000.py @@ -1161,7 +1161,7 @@ def manage_rdo(host, config): server.append('yum-config-manager --enable %(reponame)s-testing' % locals()) rc, out = server.execute() - match = re.search('enabled\s*=\s*(1|True)', out) + match = re.search(r'enabled\s*=\s*(1|True)', out) # In CentOS 7 yum-config-manager returns 0 always, but returns current setup # if succeeds # In CentOS 8 yum-config-manager returns 1 when failing but doesn't return current diff --git a/packstack/plugins/puppet_950.py b/packstack/plugins/puppet_950.py index 0f5cc4454..bd171d2a5 100755 --- a/packstack/plugins/puppet_950.py +++ b/packstack/plugins/puppet_950.py @@ -270,7 +270,7 @@ def finalize(config, messages): for hostname in filtered_hosts(config): server = utils.ScriptRunner(hostname) server.append("installed=$(rpm -q kernel --last | head -n1 | " - "sed 's/kernel-\([a-z0-9\.\_\-]*\).*/\\1/g')") + "sed 's/kernel-\([a-z0-9\.\_\-]*\).*/\\1/g')") # noqa: W605 server.append("loaded=$(uname -r | head -n1)") server.append('[ "$loaded" == "$installed" ]') try: diff --git a/packstack/plugins/ssl_001.py b/packstack/plugins/ssl_001.py index d2d01c18e..f7cf9c511 100644 --- a/packstack/plugins/ssl_001.py +++ b/packstack/plugins/ssl_001.py @@ -251,7 +251,7 @@ def create_self_signed_cert(config, messages): subject.C = config['CONFIG_SSL_CERT_SUBJECT_C'] subject.ST = config['CONFIG_SSL_CERT_SUBJECT_ST'] subject.L = config['CONFIG_SSL_CERT_SUBJECT_L'] - subject.O = config['CONFIG_SSL_CERT_SUBJECT_O'] + subject.O = config['CONFIG_SSL_CERT_SUBJECT_O'] # noqa: E741 subject.OU = config['CONFIG_SSL_CERT_SUBJECT_OU'] subject.CN = config['CONFIG_SSL_CERT_SUBJECT_CN'] subject.emailAddress = mail diff --git a/packstack/plugins/swift_600.py b/packstack/plugins/swift_600.py index 8a756b355..073af996f 100644 --- a/packstack/plugins/swift_600.py +++ b/packstack/plugins/swift_600.py @@ -232,7 +232,7 @@ def get_storage_size(config): ranges = {'G': 1048576, 'M': 1024, 'K': 1} size = config['CONFIG_SWIFT_STORAGE_SIZE'].strip() for measure in ['G', 'M', 'K']: - if re.match('\d+' + measure, size, re.IGNORECASE): + if re.match(r'\d+' + measure, size, re.IGNORECASE): intsize = int(size.rstrip(measure)) * ranges[measure] return intsize diff --git a/test-requirements.txt b/test-requirements.txt index 421af3c32..607a38254 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,7 +1,7 @@ -sphinx>=1.6.2,<2.0.0 # BSD +sphinx>=1.6.2 # BSD openstackdocstheme>=1.17.0 # Apache-2.0 reno>=0.1.1 # Apache2 stestr>=1.0.0 # Apache-2.0 coverage -hacking!=0.13.0,<0.14,>=0.12.0 +hacking>=3.1.0,<3.2.0 # Apache-2.0 mock>=2.0 # BSD diff --git a/tests/installer/test_run_setup.py b/tests/installer/test_run_setup.py index b697d3183..15f22dfa0 100644 --- a/tests/installer/test_run_setup.py +++ b/tests/installer/test_run_setup.py @@ -126,5 +126,5 @@ class CommandLineTestCase(PackstackTestCaseMixin, TestCase): sys.exit = orig_sys_exit try: shutil.rmtree(basedefs.VAR_DIR) - except: + except Exception: pass diff --git a/tests/installer/test_validators.py b/tests/installer/test_validators.py index 4e8d6ac2b..69eb1f437 100644 --- a/tests/installer/test_validators.py +++ b/tests/installer/test_validators.py @@ -40,9 +40,9 @@ class ValidatorsTestCase(PackstackTestCaseMixin, TestCase): def test_validate_regexp(self): """Test packstack.installer.validators.validate_regexp.""" - validate_regexp('Test_123', options=['\w']) + validate_regexp('Test_123', options=[r'\w']) self.assertRaises(ParamValidationError, validate_regexp, - '!#$%', options=['\w']) + '!#$%', options=[r'\w']) def test_validate_port(self): """Test packstack.installer.validators.validate_port.""" diff --git a/tox.ini b/tox.ini index 0f5772713..decd3c065 100644 --- a/tox.ini +++ b/tox.ini @@ -41,6 +41,6 @@ commands = sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasen # E123, E125 skipped as they are invalid PEP-8. # # All other checks should be enabled in the future. -ignore = E123,E125,H803,F403,F821,F811,F841,E501,H302,H303,H304,H306,H405,H404,H305,H307,H501,H201,H101 +ignore = E123,E125,H803,F403,F405,F821,F811,F841,E501,H302,H303,H304,H306,H405,H404,H305,H307,H501,H201,H101,W503,W504 show-source = True exclude=.venv,.git,.tox,.eggs