From c24c19594e4c4cddfda3226d49f8eeabacb18162 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Fri, 19 Oct 2018 08:32:52 -0700 Subject: [PATCH] Enable pylint rules and fix exposed bugs. (#47219) * Resolve invalid-unary-operand-type. * Resolve raising-format-tuple. * Resolve stop-iteration-return. * Use disable comment instead of fixing logic. The affected line in _find_address_range will only fail on Python 3.7 and later if the function is called with an empty address list. As an internal method it is never called in this way, making it a non-issue for use via public methods. Using a comment to disable the rule in favor of an ignore.txt entry since there are no plans to change the logic in the code itself. This will also prevent any potential future issues being added in other parts of the code when updating it based on upstream changes. --- lib/ansible/module_utils/compat/ipaddress.py | 2 +- lib/ansible/module_utils/pycompat24.py | 2 +- lib/ansible/modules/clustering/consul_kv.py | 2 +- lib/ansible/modules/network/cnos/cnos_backup.py | 2 +- lib/ansible/modules/network/cnos/cnos_image.py | 2 +- lib/ansible/modules/packaging/os/apt_repository.py | 1 - lib/ansible/modules/storage/netapp/netapp_e_storagepool.py | 5 ++++- test/sanity/pylint/config/default | 3 --- 8 files changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/ansible/module_utils/compat/ipaddress.py b/lib/ansible/module_utils/compat/ipaddress.py index 21a1b576fed..d6e781b926c 100644 --- a/lib/ansible/module_utils/compat/ipaddress.py +++ b/lib/ansible/module_utils/compat/ipaddress.py @@ -346,7 +346,7 @@ def _find_address_range(addresses): """ it = iter(addresses) - first = last = next(it) + first = last = next(it) # pylint: disable=stop-iteration-return for ip in it: if ip._ip != last._ip + 1: yield first, last diff --git a/lib/ansible/module_utils/pycompat24.py b/lib/ansible/module_utils/pycompat24.py index bcdf53aec58..70d0c38eeb3 100644 --- a/lib/ansible/module_utils/pycompat24.py +++ b/lib/ansible/module_utils/pycompat24.py @@ -81,7 +81,7 @@ except ImportError: if node.name in _safe_names: return _safe_names[node.name] elif isinstance(node, ast.UnarySub): - return -_convert(node.expr) + return -_convert(node.expr) # pylint: disable=invalid-unary-operand-type raise ValueError('malformed string') return _convert(node_or_string) diff --git a/lib/ansible/modules/clustering/consul_kv.py b/lib/ansible/modules/clustering/consul_kv.py index 0d7b1d1dba6..dcd46fc0c47 100644 --- a/lib/ansible/modules/clustering/consul_kv.py +++ b/lib/ansible/modules/clustering/consul_kv.py @@ -233,7 +233,7 @@ def set_value(module): value = module.params.get('value') if value is NOT_SET: - raise AssertionError('Cannot set value of "%s" to `NOT_SET`', (key, )) + raise AssertionError('Cannot set value of "%s" to `NOT_SET`' % key) index, changed = _has_value_changed(consul_api, key, value) diff --git a/lib/ansible/modules/network/cnos/cnos_backup.py b/lib/ansible/modules/network/cnos/cnos_backup.py index 6088ccd8767..a303cac9c0d 100644 --- a/lib/ansible/modules/network/cnos/cnos_backup.py +++ b/lib/ansible/modules/network/cnos/cnos_backup.py @@ -213,7 +213,7 @@ def doConfigBackUp(module, prompt, answer): elif(protocol == "tftp"): command = "copy " + configType + " " + protocol + " " + protocol command = command + "://" + server + "/" + confPath - command = command + + " vrf management\n" + command = command + " vrf management\n" # cnos.debugOutput(command) tftp_cmd = [{'command': command, 'prompt': None, 'answer': None}] cmd.extend(tftp_cmd) diff --git a/lib/ansible/modules/network/cnos/cnos_image.py b/lib/ansible/modules/network/cnos/cnos_image.py index d7da989cd80..fb17b150248 100644 --- a/lib/ansible/modules/network/cnos/cnos_image.py +++ b/lib/ansible/modules/network/cnos/cnos_image.py @@ -175,7 +175,7 @@ def doImageDownload(module, prompt, answer): elif(protocol == "tftp"): command = "copy " + protocol + " " + protocol + "://" + server command = command + "/" + imgPath + " system-image " + imgType - command = command + + " vrf management" + command = command + " vrf management" prompt = ['Confirm download operation', 'Do you want to change that to the standby image'] answer = ['y', 'y'] diff --git a/lib/ansible/modules/packaging/os/apt_repository.py b/lib/ansible/modules/packaging/os/apt_repository.py index 2d4a658822f..86f63221470 100644 --- a/lib/ansible/modules/packaging/os/apt_repository.py +++ b/lib/ansible/modules/packaging/os/apt_repository.py @@ -184,7 +184,6 @@ class SourcesList(object): for n, valid, enabled, source, comment in sources: if valid: yield file, n, enabled, source, comment - raise StopIteration def _expand_path(self, filename): if '/' in filename: diff --git a/lib/ansible/modules/storage/netapp/netapp_e_storagepool.py b/lib/ansible/modules/storage/netapp/netapp_e_storagepool.py index daa63084c41..d98cecbd2dc 100644 --- a/lib/ansible/modules/storage/netapp/netapp_e_storagepool.py +++ b/lib/ansible/modules/storage/netapp/netapp_e_storagepool.py @@ -145,7 +145,10 @@ class GroupBy(object): def _grouper(self, tgtkey): while self.currkey == tgtkey: yield self.currvalue - self.currvalue = next(self.it) # Exit on StopIteration + try: + self.currvalue = next(self.it) # Exit on StopIteration + except StopIteration: + return self.currkey = self.keyfunc(self.currvalue) diff --git a/test/sanity/pylint/config/default b/test/sanity/pylint/config/default index a0fa7c0497c..f0d324db228 100644 --- a/test/sanity/pylint/config/default +++ b/test/sanity/pylint/config/default @@ -42,7 +42,6 @@ disable= invalid-envvar-default, invalid-name, invalid-sequence-index, - invalid-unary-operand-type, keyword-arg-before-vararg, len-as-condition, line-too-long, @@ -66,7 +65,6 @@ disable= pointless-string-statement, possibly-unused-variable, protected-access, - raising-format-tuple, redefined-argument-from-local, redefined-builtin, redefined-outer-name, @@ -75,7 +73,6 @@ disable= relative-import, signature-differs, simplifiable-if-statement, - stop-iteration-return, subprocess-popen-preexec-fn, super-init-not-called, superfluous-parens,