openstack/tripleo-common
Code Issues Proposed changes

Enable pylint

Browse Source 
This is a follow-up from [1] which underlined that the code lacks even basic coverage as it allow
us to introduce a costly regression which could have being detected by pylint no-member test.

- enables pylint with most checks temporary disabled, so we have time
  to address them gradually
- fixes few minor issues reported by the tool
- adds skips for some known "no-member" errors but avoids adding it to the exclude list, as this
  in order to prevent further regressions.
- once landed we can easily address the temporary disabled errors, one by one. In fact this is could
  prove as a very good learning experience for newer team members. They can start by removing on
  random exclusion and fixing it. As a hint: leave the missing docstrings for the end, some problems
  are more important to fix first.

1: https://review.opendev.org/c/openstack/tripleo-common/+/762892/6/tripleo_common/image/builder/buildah.py

Change-Id: I10ab0cbfbaab77b9208e9a5d74d59eb041cb16ee
This commit is contained in:
Sorin Sbarnea 2020-12-16 13:58:56 +00:00 committed by Sorin Sbârnea
parent 2b3c872da0
commit 13ac87ba36
8 changed files with 89 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
.pre-commit-config.yaml
View File

@ -1,7 +1,7 @@
---
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v2.5.0
rev: v3.4.0
hooks:
- id: trailing-whitespace
- id: mixed-line-ending
@ -12,7 +12,7 @@ repos:
- id: check-yaml
files: .*\.(yaml|yml)$
- repo: https://gitlab.com/pycqa/flake8.git
rev: 3.8.1
rev: 3.8.4
hooks:
- id: flake8
- repo: https://github.com/openstack-dev/bashate.git
@ -27,3 +27,7 @@ repos:
# E040: Syntax error determined using `bash -n` (as many scripts
# use jinja templating, this will often fail and the syntax
# error will be discovered in execution anyway)
- repo: https://github.com/PyCQA/pylint
rev: pylint-2.6.0
hooks:
- id: pylint

65
.pylintrc Normal file
View File

@ -0,0 +1,65 @@
[MESSAGES CONTROL]
disable =
# TODO(ssbarnea): remove temporary skips adding during initial adoption:
arguments-differ,
attribute-defined-outside-init,
broad-except,
consider-iterating-dictionary,
consider-merging-isinstance,
consider-using-dict-comprehension,
consider-using-in,
consider-using-set-comprehension,
dangerous-default-value,
duplicate-code,
fixme,
global-statement,
import-error,
inconsistent-return-statements,
invalid-name,
logging-format-interpolation,
logging-not-lazy,
lost-exception,
missing-class-docstring,
missing-function-docstring,
missing-module-docstring,
no-else-break,
no-else-continue,
no-else-raise,
no-else-return,
no-self-use,
no-value-for-parameter,
protected-access,
raise-missing-from,
redefined-argument-from-local,
redefined-builtin,
redefined-outer-name,
simplifiable-if-statement,
super-init-not-called,
super-with-arguments,
superfluous-parens,
too-few-public-methods,
too-many-ancestors,
too-many-arguments,
too-many-branches,
too-many-instance-attributes,
too-many-lines,
too-many-locals,
too-many-nested-blocks,
too-many-public-methods,
too-many-statements,
try-except-raise,
unidiomatic-typecheck,
unnecessary-comprehension,
unnecessary-pass,
unsubscriptable-object,
unused-argument,
unused-variable,
useless-else-on-loop,
useless-object-inheritance,
useless-super-delegation,
wrong-import-order,
wrong-import-position
[REPORTS]
output-format = colorized

1
tripleo_common/exception.py
View File

@ -39,6 +39,7 @@ class TripleoCommonException(Exception):
def __init__(self, **kwargs):
self.kwargs = kwargs
self.msg_fmt = self.message
try:
self.message = self.msg_fmt % kwargs

5
tripleo_common/image/image_uploader.py
View File

@ -575,7 +575,7 @@ class ImageUploadManager(BaseImageManager):
class BaseImageUploader(object):
lock = None
mirrors = {}
insecure_registries = set()
no_verify_registries = set(NO_VERIFY_REGISTRIES)
@ -1237,11 +1237,13 @@ class BaseImageUploader(object):
name = target_image_url.path.split(':')[0][1:]
export = netloc in cls.export_registries
if export:
# pylint: disable=no-member
linked_layers = image_export.cross_repo_mount(
target_image_url, image_layers, source_layers,
uploaded_layers=cls._global_view_proxy())
# track linked layers globally for future references
for layer, info in linked_layers.items():
# pylint: disable=no-member
cls._track_uploaded_layers(
layer, known_path=info['known_path'],
image_ref=info['ref_image'], scope='local')
@ -1254,6 +1256,7 @@ class BaseImageUploader(object):
url = '%s://%s/v2/%s/blobs/uploads/' % (scheme, netloc, name)
for layer in source_layers:
# pylint: disable=no-member
known_path, existing_name = image_utils.uploaded_layers_details(
cls._global_view_proxy(), layer, scope='remote')
if layer not in image_layers and not existing_name:

4
tripleo_common/tests/image/test_image_uploader.py
View File

@ -490,6 +490,7 @@ class TestBaseImageUploader(base.TestCase):
super(TestBaseImageUploader, self).setUp()
self.uploader = image_uploader.BaseImageUploader()
self.uploader.init_registries_cache()
# pylint: disable=no-member
self.uploader._inspect.retry.sleep = mock.Mock()
self.requests = self.useFixture(rm_fixture.Fixture())
@ -1288,7 +1289,9 @@ class TestSkopeoImageUploader(base.TestCase):
def setUp(self):
super(TestSkopeoImageUploader, self).setUp()
self.uploader = image_uploader.SkopeoImageUploader()
# pylint: disable=no-member
self.uploader._copy.retry.sleep = mock.Mock()
# pylint: disable=no-member
self.uploader._inspect.retry.sleep = mock.Mock()
@mock.patch('tripleo_common.image.image_uploader.'
@ -1534,6 +1537,7 @@ class TestSkopeoImageUploader(base.TestCase):
class TestPythonImageUploader(base.TestCase):
# pylint: disable=no-member
def setUp(self):
super(TestPythonImageUploader, self).setUp()
self.uploader = image_uploader.PythonImageUploader()

8
tripleo_common/tests/utils/test_template.py
View File

@ -380,18 +380,18 @@ class ProcessTemplatesTest(base.TestCase):
self.assertTrue({'name': ['puppet/controller-role.yaml']} ==
template_utils.get_j2_excludes_file(swift))
def return_multiple_files(*args):
def return_multiple_files2(*args):
if args[1] == constants.OVERCLOUD_J2_EXCLUDES:
return ['', J2_EXCLUDES_EMPTY_LIST]
swift.get_object = mock.MagicMock(side_effect=return_multiple_files)
swift.get_object = mock.MagicMock(side_effect=return_multiple_files2)
# Test - J2 exclude file with no template to exlude
self.assertTrue(
{'name': []} == template_utils.get_j2_excludes_file(swift))
def return_multiple_files(*args):
def return_multiple_files3(*args):
if args[1] == constants.OVERCLOUD_J2_EXCLUDES:
return ['', J2_EXCLUDES_EMPTY_FILE]
swift.get_object = mock.MagicMock(side_effect=return_multiple_files)
swift.get_object = mock.MagicMock(side_effect=return_multiple_files3)
# Test - J2 exclude file empty
self.assertTrue(
{'name': []} == template_utils.get_j2_excludes_file(swift))

3
tripleo_common/utils/locks/base.py
View File

@ -15,10 +15,13 @@
class BaseLock(object):
def get_lock(self):
# pylint: disable=no-member
return self._lock
def objects(self):
# pylint: disable=no-member
return self._objects
def sessions(self):
# pylint: disable=no-member
return self._sessions

2
tripleo_common/utils/locks/processlock.py
View File

@ -26,6 +26,8 @@ class ProcessLock(base.BaseLock):
_global_view = _mgr.dict()
def __init__(self):
# https://github.com/PyCQA/pylint/issues/3313
# pylint: disable=no-member
self._lock = self._mgr.Lock()
self._objects = self._mgr.list()
self._sessions = self._mgr.dict()