Today Pylint (2.4.3)+Astroid(2.3.2) found new errors on Azure Tox task:
pylint3 create: /__w/1/s/.tox/pylint3 pylint3 installdeps: ipaclient[csrgen,otptoken_yubikey,ldap], pylint pylint3 installed: astroid==2.3.2,cffi==1.13.0,cryptography==2.8,decorator==4.4.0,dnspython==1.16.0,gssapi==1.6.1,ipaclient==4.9.0.dev201910181041+git5e1904235,ipalib==4.9.0.dev201910181041+git5e1904235,ipaplatform==4.9.0.dev201910181041+git5e1904235,ipapython==4.9.0.dev201910181041+git5e1904235,isort==4.3.21,Jinja2==2.10.3,lazy-object-proxy==1.4.2,MarkupSafe==1.1.1,mccabe==0.6.1,netaddr==0.7.19,pyasn1==0.4.7,pyasn1-modules==0.2.7,pycparser==2.19,pylint==2.4.3,python-ldap==3.2.0,python-yubico==1.3.3,pyusb==1.0.2,qrcode==6.1,six==1.12.0,typed-ast==1.4.0,wrapt==1.11.2 pylint3 run-test-pre: PYTHONHASHSEED='1555872628' pylint3 runtests: commands[0] | /__w/1/s/.tox/pylint3/bin/python -m pylint --rcfile=/__w/1/s/pylintrc --load-plugins pylint_plugins /__w/1/s/.tox/pylint3/lib/python3.7/site-packages/ipaclient /__w/1/s/.tox/pylint3/lib/python3.7/site-packages/ipalib /__w/1/s/.tox/pylint3/lib/python3.7/site-packages/ipapython ************* Module ipaclient.remote_plugins lib/python3.7/site-packages/ipaclient/remote_plugins/__init__.py:122: [C0415(import-outside-toplevel), get_package] Import outside toplevel (ipaserver)) ************* Module ipaclient.plugins.cert lib/python3.7/site-packages/ipaclient/plugins/cert.py:114: [C0415(import-outside-toplevel), cert_request.forward] Import outside toplevel (ipaclient)) ************* Module ipaclient.plugins.csrgen lib/python3.7/site-packages/ipaclient/plugins/csrgen.py:77: [C0415(import-outside-toplevel), cert_get_requestdata.execute] Import outside toplevel (ipaclient)) lib/python3.7/site-packages/ipaclient/plugins/csrgen.py:78: [C0415(import-outside-toplevel), cert_get_requestdata.execute] Import outside toplevel (ipaclient)) ************* Module ipapython.cookie lib/python3.7/site-packages/ipapython/cookie.py:596: [C0415(import-outside-toplevel), Cookie.http_return_ok.domain_valid] Import outside toplevel (ipalib.util))
All these warnings can and should be ignored
What is about global policy for import-outside-toplevel?
import-outside-toplevel
This is the full report of make lint on master against Pylint 2.4.3:
make lint
Pylint on /usr/bin/python3 is running, please wait ... ************* Module makeaci makeaci:102: [C0415(import-outside-toplevel), main] Import outside toplevel (ipaserver.install.plugins)) ************* Module ipapython.cookie ipapython/cookie.py:596: [C0415(import-outside-toplevel), Cookie.http_return_ok.domain_valid] Import outside toplevel (ipalib.util)) ************* Module ipapython.install.common ipapython/install/common.py:38: [W0125(using-constant-test), Installable._get_components] Using a conditional statement with a constant value) ipapython/install/common.py:43: [W0125(using-constant-test), Installable._configure] Using a conditional statement with a constant value) ************* Module ipaclient.remote_plugins ipaclient/remote_plugins/__init__.py:122: [C0415(import-outside-toplevel), get_package] Import outside toplevel (ipaserver)) ************* Module ipaclient.install.ipa_certupdate ipaclient/install/ipa_certupdate.py:112: [C0415(import-outside-toplevel), run_with_args] Import outside toplevel (ipaserver.install)) ************* Module ipaclient.plugins.cert ipaclient/plugins/cert.py:114: [C0415(import-outside-toplevel), cert_request.forward] Import outside toplevel (ipaclient)) ************* Module ipaclient.plugins.csrgen ipaclient/plugins/csrgen.py:77: [C0415(import-outside-toplevel), cert_get_requestdata.execute] Import outside toplevel (ipaclient)) ipaclient/plugins/csrgen.py:78: [C0415(import-outside-toplevel), cert_get_requestdata.execute] Import outside toplevel (ipaclient)) ************* Module ipaplatform.redhat.tasks ipaplatform/redhat/tasks.py:307: [C0415(import-outside-toplevel), RedHatTaskNamespace.insert_ca_certs_into_systemwide_ca_store] Import outside toplevel (ipalib)) ipaplatform/redhat/tasks.py:308: [C0415(import-outside-toplevel), RedHatTaskNamespace.insert_ca_certs_into_systemwide_ca_store] Import outside toplevel (ipalib.errors)) ipaplatform/redhat/tasks.py:639: [C0415(import-outside-toplevel), RedHatTaskNamespace.configure_dns_resolver] Import outside toplevel (ipaplatform.services)) ipaplatform/redhat/tasks.py:684: [C0415(import-outside-toplevel), RedHatTaskNamespace.unconfigure_dns_resolver] Import outside toplevel (ipaplatform.services)) ************* Module ipaplatform.redhat.services ipaplatform/redhat/services.py:225: [C0415(import-outside-toplevel), RedHatServices.__init__] Import outside toplevel (ipalib)) ************* Module ipaplatform.debian.services ipaplatform/debian/services.py:168: [C0415(import-outside-toplevel), DebianServices.__init__] Import outside toplevel (ipalib)) ************* Module ipaplatform.base.services ipaplatform/base/services.py:113: [C0415(import-outside-toplevel), PlatformService.__init__] Import outside toplevel (ipalib)) ************* Module makeapi makeapi:87: [C0415(import-outside-toplevel), parse_options] Import outside toplevel (optparse)) ************* Module ipa-csreplica-manage install/tools/ipa-csreplica-manage:56: [C0415(import-outside-toplevel), parse_options] Import outside toplevel (optparse)) ************* Module ipaserver.advise.base ipaserver/advise/base.py:431: [C0415(import-outside-toplevel), AdviseAPI.packages] Import outside toplevel (ipaserver.advise.plugins)) ************* Module ipaserver.install.ldapupdate ipaserver/install/ldapupdate.py:521: [R1724(no-else-continue), LDAPUpdate.parse_update_file] Unnecessary "else" after "continue") ************* Module ipaserver.install.service ipaserver/install/service.py:462: [W0128(redeclared-assigned-name), Service.export_ca_certs_file] Redeclared variable '_unused' in assignment) ************* Module ipaserver.install.dogtaginstance ipaserver/install/dogtaginstance.py:936: [C0415(import-outside-toplevel), test] Import outside toplevel (sys)) ************* Module ipaserver.install.server.replicainstall ipaserver/install/server/replicainstall.py:495: [R1724(no-else-continue), promote_openldap_conf] Unnecessary "elif" after "continue") ************* Module ipaserver.install.server ipaserver/install/server/__init__.py:455: [W0125(using-constant-test), ServerInstallInterface.__init__] Using a conditional statement with a constant value) ************* Module ipaserver.install.plugins.adtrust ipaserver/install/plugins/adtrust.py:813: [R1723(no-else-break), update_host_cifs_keytabs.execute] Unnecessary "else" after "break") ************* Module ipaserver.install.plugins.update_managed_permissions ipaserver/install/plugins/update_managed_permissions.py:560: [R1724(no-else-continue), update_managed_permissions.get_upgrade_attr_lists] Unnecessary "else" after "continue") ************* Module ipaserver.plugins.delegation ipaserver/plugins/delegation.py:116: [R1721(unnecessary-comprehension), delegation.__json__] Unnecessary use of a comprehension) ************* Module ipaserver.plugins.config ipaserver/plugins/config.py:498: [W0128(redeclared-assigned-name), config_mod.pre_callback] Redeclared variable '_dummy' in assignment) ipaserver/plugins/config.py:530: [W0128(redeclared-assigned-name), config_mod.pre_callback] Redeclared variable '_dummy' in assignment) ************* Module ipaserver.plugins.selfservice ipaserver/plugins/selfservice.py:108: [R1721(unnecessary-comprehension), selfservice.__json__] Unnecessary use of a comprehension) ************* Module ipaserver.plugins.trust ipaserver/plugins/trust.py:363: [R1723(no-else-break), add_range] Unnecessary "else" after "break") ************* Module ipasetup ipasetup.py:171: [C0415(import-outside-toplevel), ipasetup] Import outside toplevel (setuptools)) ************* Module ipatests.pytest_ipa.integration.config ipatests/pytest_ipa/integration/config.py:96: [C0415(import-outside-toplevel), Config.from_env] Import outside toplevel (ipatests.pytest_ipa.integration.env_config)) ipatests/pytest_ipa/integration/config.py:100: [C0415(import-outside-toplevel), Config.to_env] Import outside toplevel (ipatests.pytest_ipa.integration.env_config)) ipatests/pytest_ipa/integration/config.py:152: [C0415(import-outside-toplevel), Domain.get_host_class] Import outside toplevel (ipatests.pytest_ipa.integration.host)) ipatests/pytest_ipa/integration/config.py:187: [C0415(import-outside-toplevel), Domain.from_env] Import outside toplevel (ipatests.pytest_ipa.integration.env_config)) ipatests/pytest_ipa/integration/config.py:191: [C0415(import-outside-toplevel), Domain.to_env] Import outside toplevel (ipatests.pytest_ipa.integration.env_config)) ************* Module ipatests.pytest_ipa.integration.tasks ipatests/pytest_ipa/integration/tasks.py:814: [C0415(import-outside-toplevel), modify_sssd_conf] Import outside toplevel (SSSDConfig)) ipatests/pytest_ipa/integration/tasks.py:1369: [R1723(no-else-break), wait_for_cleanallruv_tasks] Unnecessary "else" after "break") ************* Module ipatests.pytest_ipa.integration.host ipatests/pytest_ipa/integration/host.py:100: [C0415(import-outside-toplevel), Host.from_env] Import outside toplevel (ipatests.pytest_ipa.integration.env_config)) ipatests/pytest_ipa/integration/host.py:104: [C0415(import-outside-toplevel), Host.to_env] Import outside toplevel (ipatests.pytest_ipa.integration.env_config)) ************* Module ipatests.pytest_ipa.integration.env_configinternal error with sending report for module ['ipaserver/plugins/serverroles.py'] maximum recursion depth exceeded while calling a Python object ipatests/pytest_ipa/integration/env_config.py:115: [C0415(import-outside-toplevel), config_from_env] Import outside toplevel (yaml)) ************* Module ipatests.test_xmlrpc.test_certmap_plugin ipatests/test_xmlrpc/test_certmap_plugin.py:209: [R1721(unnecessary-comprehension), addcertmap_id] Unnecessary use of a comprehension) ************* Module ipatests.test_xmlrpc.test_cert_plugin ipatests/test_xmlrpc/test_cert_plugin.py:242: [C0415(import-outside-toplevel), test_cert.test_00011_emails_are_valid] Import outside toplevel (ipaserver.plugins.cert)) ************* Module ipatests.test_integration.test_caless ipatests/test_integration/test_caless.py:133: [W0125(using-constant-test), CALessBase.install] Using a conditional statement with a constant value) ipatests/test_integration/test_caless.py:137: [W0125(using-constant-test), CALessBase.install] Using a conditional statement with a constant value) ************* Module ipatests.test_ipatests_plugins.test_slicing ipatests/test_ipatests_plugins/test_slicing.py:42: [R1721(unnecessary-comprehension), test_slicing] Unnecessary use of a comprehension) ipatests/test_ipatests_plugins/test_slicing.py:43: [R1721(unnecessary-comprehension), test_slicing] Unnecessary use of a comprehension) ipatests/test_ipatests_plugins/test_slicing.py:44: [R1721(unnecessary-comprehension), test_slicing] Unnecessary use of a comprehension) ************* Module ipalib.install.certmonger ipalib/install/certmonger.py:346: [R1723(no-else-break), request_and_wait_for_cert] Unnecessary "elif" after "break") ------------------------------------
We try to avoid it where possible. When necessary the reasons should be documented or obvious.
I spot-checked a couple and this was true in most cases (makeaci less so). Some are due to avoiding import loops, others for deferring for optional features, etc.
IMHO we can ignore this and continue to catch during reviews.
I'm sorry, ignore globally or spotted?
Ignore globally.
We can catch them during reviews. If it becomes a problem we can reconsider later and go back and add exceptions.
master:
Metadata Update from @ftweedal: - Issue close_status updated to: fixed - Issue status updated to: Closed (was: Open)
Please backport the fix to 4.8 branch, too.
Metadata Update from @cheimes: - Issue status updated to: Open (was: Closed)
Metadata Update from @fcami: - Custom field on_review adjusted to https://github.com/freeipa/freeipa/pull/3815
hi @slev Could you please backport this to the ipa-4-8 branch if at all possible?
There is same issue in ipa-4-7 and ipa-4-6
ipa-4-8:
ipa-4-7:
ipa-4-6:
Metadata Update from @rcritten: - Issue close_status updated to: fixed - Issue status updated to: Closed (was: Open)
@fcami, @rcritten, hi. Sorry, I had a vacation.
Actually, there are still several errors: ipa-4-7:
ipa-4-7
Pylint on /usr/bin/python3 is running, please wait ... ************* Module ipalib.install.certmonger ipalib/install/certmonger.py:342: [R1723(no-else-break), request_and_wait_for_cert] Unnecessary "elif" after "break") ************* Module ipaserver.dcerpc ipaserver/dcerpc.py:823: [R1721(unnecessary-comprehension), string_to_array] Unnecessary use of a comprehension) ************* Module ipaserver.plugins.dns ipaserver/plugins/dns.py:718: [R1724(no-else-continue), DNSRecord._part_values_to_string] Unnecessary "elif" after "continue") ipaserver/plugins/dns.py:756: [W0125(using-constant-test), DNSRecord.normalize] Using a conditional statement with a constant value) ipaserver/plugins/dns.py:3440: [R1724(no-else-continue), dnsrecord.wait_for_modified_attrs] Unnecessary "else" after "continue") ipaserver/plugins/dns.py:3849: [R1724(no-else-continue), dnsrecord_del.get_options] Unnecessary "elif" after "continue") ipaserver/plugins/dns.py:3979: [R1724(no-else-continue), dnsrecord_find.get_options] Unnecessary "elif" after "continue") ************* Module ipaserver.plugins.baseldap ipaserver/plugins/baseldap.py:815: [R1721(unnecessary-comprehension), LDAPObject.__json__] Unnecessary use of a comprehension) ************* Module ipaserver.plugins.automember ipaserver/plugins/automember.py:810: [R1723(no-else-break), automember_rebuild.execute] Unnecessary "else" after "break") ************* Module ipaclient.install.ipachangeconf ipaclient/install/ipachangeconf.py:186: [R1721(unnecessary-comprehension), IPAChangeConf.dump] Unnecessary use of a comprehension) ------------------------------------ Your code has been rated at 10.00/10 make: *** [Makefile:1267: pylint] Error 12
ipa-4-6
Pylint on /usr/bin/python3 is running, please wait ... ************* Module ipatests.test_util ipatests/test_util.py:86: [E1101(no-member), test_Fuzzy.test_init] Module 're' has no '_pattern_type' member) ipatests/test_util.py:92: [E1101(no-member), test_Fuzzy.test_init] Module 're' has no '_pattern_type' member) ************* Module ipatests.test_xmlrpc.test_certmap_plugin ipatests/test_xmlrpc/test_certmap_plugin.py:348: [W1656(dict-values-not-iterating), certmap_user_permissions] dict.values referenced when not iterating) ************* Module ipatests.test_ipapython.test_ipautil ipatests/test_ipapython/test_ipautil.py:179: [W1620(dict-iter-method), TestCIDict.test_items] Calling a dict.iter*() method) ipatests/test_ipapython/test_ipautil.py:180: [W1655(dict-keys-not-iterating), TestCIDict.test_items] dict.keys referenced when not iterating) ipatests/test_ipapython/test_ipautil.py:180: [W1656(dict-values-not-iterating), TestCIDict.test_items] dict.values referenced when not iterating) ipatests/test_ipapython/test_ipautil.py:188: [W1620(dict-iter-method), TestCIDict.test_iteritems] Calling a dict.iter*() method) ipatests/test_ipapython/test_ipautil.py:198: [W1620(dict-iter-method), TestCIDict.test_iterkeys] Calling a dict.iter*() method) ipatests/test_ipapython/test_ipautil.py:208: [W1620(dict-iter-method), TestCIDict.test_itervalues] Calling a dict.iter*() method) ipatests/test_ipapython/test_ipautil.py:224: [W1620(dict-iter-method), TestCIDict.test_keys] Calling a dict.iter*() method) ipatests/test_ipapython/test_ipautil.py:234: [W1620(dict-iter-method), TestCIDict.test_values] Calling a dict.iter*() method) ************* Module ipalib.install.certmonger ipalib/install/certmonger.py:349: [R1723(no-else-break), request_and_wait_for_cert] Unnecessary "else" after "break") ************* Module ipaserver.dcerpc ipaserver/dcerpc.py:835: [R1721(unnecessary-comprehension), string_to_array] Unnecessary use of a comprehension) ************* Module ipaserver.plugins.dns ipaserver/plugins/dns.py:718: [R1724(no-else-continue), DNSRecord._part_values_to_string] Unnecessary "elif" after "continue") ipaserver/plugins/dns.py:756: [W0125(using-constant-test), DNSRecord.normalize] Using a conditional statement with a constant value) ipaserver/plugins/dns.py:3440: [R1724(no-else-continue), dnsrecord.wait_for_modified_attrs] Unnecessary "else" after "continue") ipaserver/plugins/dns.py:3850: [R1724(no-else-continue), dnsrecord_del.get_options] Unnecessary "elif" after "continue") ipaserver/plugins/dns.py:3980: [R1724(no-else-continue), dnsrecord_find.get_options] Unnecessary "elif" after "continue") ************* Module ipaserver.plugins.baseldap ipaserver/plugins/baseldap.py:815: [R1721(unnecessary-comprehension), LDAPObject.__json__] Unnecessary use of a comprehension) ************* Module ipaserver.plugins.automember ipaserver/plugins/automember.py:810: [R1723(no-else-break), automember_rebuild.execute] Unnecessary "else" after "break") ************* Module ipaserver.install.installutils ipaserver/install/installutils.py:790: [W0706(try-except-raise), encrypt_file] The except handler raises immediately) ipaserver/install/installutils.py:826: [W0706(try-except-raise), decrypt_file] The except handler raises immediately) ************* Module ipaserver.install.ipa_cert_fix ipaserver/install/ipa_cert_fix.py:71: [R1710(inconsistent-return-statements), IPACertFix.run] Either all return statements in a function should return an expression, or none of them should.) ************* Module ipa-dnskeysync-replica daemons/dnssec/ipa-dnskeysync-replica:53: [R1710(inconsistent-return-statements), find_unwrapping_key] Either all return statements in a function should return an expression, or none of them should.) ************* Module ipaclient.install.ipachangeconf ipaclient/install/ipachangeconf.py:183: [R1721(unnecessary-comprehension), IPAChangeConf.dump] Unnecessary use of a comprehension)
Some of them are related to original PR and some are not. So, cherry-picking from the corresponding pylint PRs would be enough.
pylint
Does ipa-4-{6,7} require these fixes at all? pylint in these branches is too old and cann't catch new 'problems'.
Hi @slev I did a minimal backport of your fix from master to ipa-4-7 and ipa-4-6, in order to have PR tests pass. If the additional reported issues are potential bugs, then yes, we should backport the fixes. If they are only refactoring advices, I think the old branches can live with them, as long as PRCI is green.
Hi, @frenaud, I wouldn't like to touch them too. Thanks.