From 7d0fbbf41090ab3a614be129d605b8e453dab36b Mon Sep 17 00:00:00 2001 From: Stanislav Levin Date: Nov 04 2019 19:24:00 +0000 Subject: Fix errors found by Pylint-2.4.3 New Pylint (2.4.3) catches several new 'true problems'. At the same time, it warns about things that are massively and reasonably employed in FreeIPA. list of fixed: - no-else-continue - redeclared-assigned-name - no-else-break - unnecessary-comprehension - using-constant-test (false positive) list of ignored (responsibility of contributors and reviewers): - import-outside-toplevel Fixes: https://pagure.io/freeipa/issue/8102 Signed-off-by: Stanislav Levin Reviewed-By: Fraser Tweedale --- diff --git a/ipaclient/discovery.py b/ipaclient/discovery.py index 2edbf2c..c322e97 100644 --- a/ipaclient/discovery.py +++ b/ipaclient/discovery.py @@ -605,11 +605,9 @@ class IPADiscovery: def main(): - # pylint: disable=import-outside-toplevel import argparse import os from ipapython.ipa_log_manager import standard_logging_setup - # pylint: enable=import-outside-toplevel parser = argparse.ArgumentParser(__name__) diff --git a/ipalib/__init__.py b/ipalib/__init__.py index 36f5765..094fc5e 100644 --- a/ipalib/__init__.py +++ b/ipalib/__init__.py @@ -886,10 +886,8 @@ from ipapython.version import VERSION as __version__ def _enable_warnings(error=False): """Enable additional warnings during development """ - # pylint: disable=import-outside-toplevel import ctypes import warnings - # pylint: enable=import-outside-toplevel # get reference to Py_BytesWarningFlag from Python CAPI byteswarnings = ctypes.c_int.in_dll( # pylint: disable=no-member @@ -939,10 +937,8 @@ class API(plugable.API): def packages(self): if self.env.in_server: # pylint: disable=import-error,ipa-forbidden-import - # pylint: disable=import-outside-toplevel import ipaserver.plugins # pylint: enable=import-error,ipa-forbidden-import - # pylint: enable=import-outside-toplevel result = ( ipaserver.plugins, ) @@ -952,10 +948,8 @@ class API(plugable.API): # https://github.com/PyCQA/pylint/issues/872 # Thus, below line was added as a workaround result = None - # pylint: disable=import-outside-toplevel import ipaclient.remote_plugins import ipaclient.plugins - # pylint: enable=import-outside-toplevel result = ( ipaclient.remote_plugins.get_package(self), ipaclient.plugins, @@ -963,10 +957,8 @@ class API(plugable.API): if self.env.context in ('installer', 'updates'): # pylint: disable=import-error,ipa-forbidden-import - # pylint: disable=import-outside-toplevel import ipaserver.install.plugins # pylint: enable=import-error,ipa-forbidden-import - # pylint: enable=import-outside-toplevel result += (ipaserver.install.plugins,) return result diff --git a/ipalib/install/certmonger.py b/ipalib/install/certmonger.py index 425234c..33eaa91 100644 --- a/ipalib/install/certmonger.py +++ b/ipalib/install/certmonger.py @@ -347,13 +347,13 @@ def request_and_wait_for_cert( # probably unrecoverable error logger.debug("Giving up on cert request %s", req_id) break - elif not resubmit_timeout: + if not resubmit_timeout: # no resubmit break - elif time.time() > deadline: + if time.time() > deadline: logger.debug("Request %s reached resubmit deadline", req_id) break - elif state == 'TIMEOUT': + if state == 'TIMEOUT': logger.debug("%s not in final state, continue waiting", req_id) time.sleep(10) else: diff --git a/ipapython/install/common.py b/ipapython/install/common.py index eaf78e3..ff264e5 100644 --- a/ipapython/install/common.py +++ b/ipapython/install/common.py @@ -35,12 +35,12 @@ class Installable(core.Configurable): def _get_components(self): components = super(Installable, self)._get_components() - if self.uninstalling: + if self.uninstalling: # pylint: disable=using-constant-test components = reversed(list(components)) return components def _configure(self): - if self.uninstalling: + if self.uninstalling: # pylint: disable=using-constant-test return self._uninstall() else: return self._install() diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py index 0cdea6a..7ce770a 100644 --- a/ipaserver/install/ldapupdate.py +++ b/ipaserver/install/ldapupdate.py @@ -521,9 +521,8 @@ class LDAPUpdate: if source_line.startswith(' '): logical_line += source_line[1:] continue - else: - emit_item(logical_line) - logical_line = source_line + emit_item(logical_line) + logical_line = source_line if dn is not None: emit_item(logical_line) diff --git a/ipaserver/install/plugins/adtrust.py b/ipaserver/install/plugins/adtrust.py index 386fe53..b7bb53f 100644 --- a/ipaserver/install/plugins/adtrust.py +++ b/ipaserver/install/plugins/adtrust.py @@ -815,8 +815,7 @@ class update_host_cifs_keytabs(Updater): self.copy_key(paths.SAMBA_KEYTAB, hostkey) copied = True break - else: - uptodate = True + uptodate = True if not (copied or uptodate): self.copy_key(paths.SAMBA_KEYTAB, hostkey) diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py index 9457e87..11cbfa8 100644 --- a/ipaserver/install/plugins/update_managed_permissions.py +++ b/ipaserver/install/plugins/update_managed_permissions.py @@ -560,8 +560,7 @@ class update_managed_permissions(Updater): if current_aci != default_aci: logger.debug('ACIs not compatible') continue - else: - all_incompatible = False + all_incompatible = False if attrs_in_all_defaults is None: attrs_in_all_defaults = set(default_attrs) diff --git a/ipaserver/install/server/__init__.py b/ipaserver/install/server/__init__.py index ac56ddc..8c6a26b 100644 --- a/ipaserver/install/server/__init__.py +++ b/ipaserver/install/server/__init__.py @@ -452,7 +452,7 @@ class ServerInstallInterface(ServerCertificateInstallInterface, "You cannot specify --external-ca-profile without " "--external-ca") - if self.uninstalling: + if self.uninstalling: # pylint: disable=using-constant-test if (self.realm_name or self.admin_password or self.master_password): raise RuntimeError( diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index 9b273c5..536f0db 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -494,7 +494,7 @@ def promote_openldap_conf(hostname, master): for opt in old_opts: if opt['type'] == 'comment' and master in opt['value']: continue - elif (opt['type'] == 'option' and opt['name'] == 'URI' and + if (opt['type'] == 'option' and opt['name'] == 'URI' and master in opt['value']): continue new_opts.append(opt) diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py index 7ba6255..eee2939 100644 --- a/ipaserver/install/service.py +++ b/ipaserver/install/service.py @@ -459,7 +459,7 @@ class Service: pass else: with open(cafile, 'wb') as fd: - for cert, _unused, _unused, _unused in ca_certs: + for cert, _unused1, _unused2, _unused3 in ca_certs: fd.write(cert.public_bytes(x509.Encoding.PEM)) def export_ca_certs_nssdb(self, db, ca_is_configured, conn=None): diff --git a/ipaserver/plugins/config.py b/ipaserver/plugins/config.py index ba39d45..e96e519 100644 --- a/ipaserver/plugins/config.py +++ b/ipaserver/plugins/config.py @@ -495,7 +495,7 @@ class config_mod(LDAPUpdate): for field in entry_attrs[k].split(',')] # test if all base types (without sub-types) are allowed for a in attributes: - a, _dummy, _dummy = a.partition(';') + a, _unused1, _unused2 = a.partition(';') if a not in allowed_attrs: raise errors.ValidationError( name=k, error=_('attribute "%s" not allowed') % a @@ -527,7 +527,7 @@ class config_mod(LDAPUpdate): if self.api.Object[obj].uuid_attribute: checked_attrs = checked_attrs + [self.api.Object[obj].uuid_attribute] for obj_attr in checked_attrs: - obj_attr, _dummy, _dummy = obj_attr.partition(';') + obj_attr, _unused1, _unused2 = obj_attr.partition(';') if obj_attr in OPERATIONAL_ATTRIBUTES: continue if obj_attr in self.api.Object[obj].params and \ diff --git a/ipaserver/plugins/delegation.py b/ipaserver/plugins/delegation.py index 1e9665b..afabdda 100644 --- a/ipaserver/plugins/delegation.py +++ b/ipaserver/plugins/delegation.py @@ -113,7 +113,7 @@ class delegation(Object): ) json_dict['primary_key'] = self.primary_key.name - json_dict['methods'] = [m for m in self.methods] + json_dict['methods'] = list(self.methods) return json_dict def postprocess_result(self, result): diff --git a/ipaserver/plugins/selfservice.py b/ipaserver/plugins/selfservice.py index a836757..eb6e258 100644 --- a/ipaserver/plugins/selfservice.py +++ b/ipaserver/plugins/selfservice.py @@ -105,7 +105,7 @@ class selfservice(Object): (a, getattr(self, a)) for a in json_friendly_attributes ) json_dict['primary_key'] = self.primary_key.name - json_dict['methods'] = [m for m in self.methods] + json_dict['methods'] = list(self.methods) return json_dict def postprocess_result(self, result): diff --git a/ipaserver/plugins/trust.py b/ipaserver/plugins/trust.py index 8b4561f..1046c4c 100644 --- a/ipaserver/plugins/trust.py +++ b/ipaserver/plugins/trust.py @@ -363,8 +363,7 @@ def add_range(myapi, trustinstance, range_name, dom_sid, *keys, **options): if info_list: info = info_list[0] break - else: - sleep(2) + sleep(2) required_msSFU_attrs = ['msSFU30MaxUidNumber', 'msSFU30OrderNumber'] diff --git a/ipatests/pytest_ipa/integration/tasks.py b/ipatests/pytest_ipa/integration/tasks.py index ffa89a2..85f9e02 100644 --- a/ipatests/pytest_ipa/integration/tasks.py +++ b/ipatests/pytest_ipa/integration/tasks.py @@ -1350,9 +1350,8 @@ def wait_for_cleanallruv_tasks(ldap, timeout=30): ): logger.debug("All cleanallruv tasks finished successfully") break - else: - logger.debug("cleanallruv task in progress, (waited %s/%ss)", - i, timeout) + logger.debug("cleanallruv task in progress, (waited %s/%ss)", + i, timeout) time.sleep(1) else: logger.error('Giving up waiting for cleanallruv to finish') diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py index 9a853e0..ab0c886 100644 --- a/ipatests/test_integration/test_caless.py +++ b/ipatests/test_integration/test_caless.py @@ -130,11 +130,11 @@ class CALessBase(IntegrationTest): cls.cert_password = cls.master.config.admin_password cls.crl_path = os.path.join(cls.master.config.test_dir, 'crl') - if cls.replicas: + if cls.replicas: # pylint: disable=using-constant-test replica_hostname = cls.replicas[0].hostname else: replica_hostname = 'unused-replica.test' - if cls.clients: + if cls.clients: # pylint: disable=using-constant-test client_hostname = cls.clients[0].hostname else: client_hostname = 'unused-client.test' diff --git a/ipatests/test_ipatests_plugins/test_slicing.py b/ipatests/test_ipatests_plugins/test_slicing.py index e21af33..a663772 100644 --- a/ipatests/test_ipatests_plugins/test_slicing.py +++ b/ipatests/test_ipatests_plugins/test_slicing.py @@ -39,9 +39,9 @@ def ipatestdir(testdir): "nslices,nslices_d,groups", [(2, 0, [[x for x in range(MODS_NUM) if x % 2 == 0], [x for x in range(MODS_NUM) if x % 2 != 0]]), - (2, 1, [[0], [x for x in range(1, MODS_NUM)]]), - (1, 0, [[x for x in range(MODS_NUM)]]), - (1, 1, [[x for x in range(MODS_NUM)]]), + (2, 1, [[0], list(range(1, MODS_NUM))]), + (1, 0, [list(range(MODS_NUM))]), + (1, 1, [list(range(MODS_NUM))]), (MODS_NUM, MODS_NUM, [[x] for x in range(MODS_NUM)]), ]) def test_slicing(ipatestdir, nslices, nslices_d, groups): diff --git a/ipatests/test_xmlrpc/test_certmap_plugin.py b/ipatests/test_xmlrpc/test_certmap_plugin.py index ce88ced..0fb574e 100644 --- a/ipatests/test_xmlrpc/test_certmap_plugin.py +++ b/ipatests/test_xmlrpc/test_certmap_plugin.py @@ -206,7 +206,7 @@ def certmap_user(request): def addcertmap_id(options): if options: - return u', '.join([k for k in options]) + return u', '.join(list(options)) else: return u' ' diff --git a/pylintrc b/pylintrc index 741e52d..7f114b7 100644 --- a/pylintrc +++ b/pylintrc @@ -101,6 +101,7 @@ disable= keyword-arg-before-vararg, # pylint 2.0, remove after dropping Python 2 consider-using-enumerate, # pylint 2.1, clean up tests later no-else-raise, # python 2.4.0 + import-outside-toplevel, # pylint 2.4.2 [REPORTS]