#8102 Pylint 2.4.3 + Astroid 2.3.2 errors
Closed: fixed by rcritten. Opened by slev.

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?

This is the full report of make lint on master against Pylint 2.4.3:

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")
------------------------------------

What is about global policy for import-outside-toplevel?

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.

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:

  • c6769ad12f79c9430ecf7c1a97be8b865a059e23 (HEAD) Fix errors found by Pylint-2.4.3

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:

  • 7d0fbbf41090ab3a614be129d605b8e453dab36b Fix errors found by Pylint-2.4.3

ipa-4-7:

  • 20548ef82f470e11c11722b549f4aac21e8ca5a7 Fix errors found by Pylint-2.4.3

ipa-4-6:

  • f0f839326c8c0de83cb875a473b3fb5d4a014296 Fix errors found by Pylint-2.4.3

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:

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.

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.

Metadata