From 501fa35af2780d57329478fe0ef0a6037ef8bf73 Mon Sep 17 00:00:00 2001 From: Al Bailey Date: Tue, 21 Jan 2020 17:04:56 -0600 Subject: [PATCH] Update pylint for distributedcloud The pylint that was specified as an upper constraint was not as strict as the newer versions. - Fixed or suppressed any new error codes being reported. Suppressed error codes should be fixed by additional reviews. - added a zuul target for pylint - Added dcdbsync to the folders being processed by pylint. - Removed the env:UPPER_CONSTRAINTS_FILE from tox.ini so that zuul does not override the upper constraints with an newer version. - Needed to add greenlet to the extension-pkg-whitelist since runtime introspection of that component is not possible. - oslo-messaging get_transport deprecated 'aliases' in pike, and removed it in rocky. The packaged version in STX is 5.30.6 which still supports aliases. This requires us to explicitly run tox with the version that we ship (pre-stein). This is controlled by a local upper-constraints.txt file. Related-Bug: 1865054 - Paste file handling reports a pylint issue which has been suppressed Related-Bug: 1865085 Story: 2004515 Task: 38882 Change-Id: Ie7c8c2adab1eeb9100159a2aa29968a0b557e2e4 Signed-off-by: Al Bailey --- .zuul.yaml | 15 ++++++++++ .../dcdbsync/dbsyncclient/httpclient.py | 10 +++---- distributedcloud/dcmanager/common/utils.py | 7 +++-- .../dcmanager/manager/subcloud_install.py | 2 +- .../dcorch/api/proxy/common/service.py | 3 +- .../dcorch/drivers/openstack/sysinv_v1.py | 4 +-- .../dcorch/engine/fernet_key_manager.py | 2 +- .../dcorch/engine/sync_services/sysinv.py | 6 ++-- distributedcloud/pylint.rc | 30 ++++++++++++++----- distributedcloud/test-requirements.txt | 4 +-- distributedcloud/tox.ini | 18 ++++++----- distributedcloud/upper-constraints.txt | 4 +++ tox.ini | 2 +- 13 files changed, 72 insertions(+), 35 deletions(-) create mode 100644 distributedcloud/upper-constraints.txt diff --git a/.zuul.yaml b/.zuul.yaml index 72bc4ffbd..ee2a636a0 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -9,11 +9,13 @@ - openstack-tox-linters - stx-distcloud-tox-pep8 - stx-distcloud-tox-py27 + - stx-distcloud-tox-pylint gate: jobs: - openstack-tox-linters - stx-distcloud-tox-pep8 - stx-distcloud-tox-py27 + - stx-distcloud-tox-pylint post: jobs: - stx-distcloud-upload-git-mirror @@ -31,6 +33,19 @@ tox_envlist: py27 tox_extra_args: -c distributedcloud/tox.ini +- job: + name: stx-distcloud-tox-pylint + parent: tox + description: Run pylint for distcloud + required-projects: + - starlingx/fault + - starlingx/nfv + - starlingx/update + - starlingx/config + vars: + tox_envlist: pylint + tox_extra_args: -c distributedcloud/tox.ini + - job: name: stx-distcloud-tox-pep8 parent: tox diff --git a/distributedcloud/dcdbsync/dbsyncclient/httpclient.py b/distributedcloud/dcdbsync/dbsyncclient/httpclient.py index 9f405794f..e16cd5da1 100644 --- a/distributedcloud/dcdbsync/dbsyncclient/httpclient.py +++ b/distributedcloud/dcdbsync/dbsyncclient/httpclient.py @@ -81,7 +81,7 @@ class HTTPClient(object): raise exceptions.ConnectFailure(msg) except requests.exceptions.RequestException as e: msg = 'Unexpected exception for %s: %s' % (url, e) - raise exceptions.UnknownConnectionError(msg, e) + raise exceptions.UnknownConnectionError(msg) @log_request def post(self, url, body, headers=None): @@ -99,7 +99,7 @@ class HTTPClient(object): raise exceptions.ConnectFailure(msg) except requests.exceptions.RequestException as e: msg = 'Unexpected exception for %s: %s' % (url, e) - raise exceptions.UnknownConnectionError(msg, e) + raise exceptions.UnknownConnectionError(msg) @log_request def put(self, url, body, headers=None): @@ -117,7 +117,7 @@ class HTTPClient(object): raise exceptions.ConnectFailure(msg) except requests.exceptions.RequestException as e: msg = 'Unexpected exception for %s: %s' % (url, e) - raise exceptions.UnknownConnectionError(msg, e) + raise exceptions.UnknownConnectionError(msg) @log_request def patch(self, url, body, headers=None): @@ -135,7 +135,7 @@ class HTTPClient(object): raise exceptions.ConnectFailure(msg) except requests.exceptions.RequestException as e: msg = 'Unexpected exception for %s: %s' % (url, e) - raise exceptions.UnknownConnectionError(msg, e) + raise exceptions.UnknownConnectionError(msg) @log_request def delete(self, url, headers=None): @@ -153,7 +153,7 @@ class HTTPClient(object): raise exceptions.ConnectFailure(msg) except requests.exceptions.RequestException as e: msg = 'Unexpected exception for %s: %s' % (url, e) - raise exceptions.UnknownConnectionError(msg, e) + raise exceptions.UnknownConnectionError(msg) def _get_request_options(self, method, headers): headers = self._update_headers(headers) diff --git a/distributedcloud/dcmanager/common/utils.py b/distributedcloud/dcmanager/common/utils.py index 2a5f33605..7baf04881 100644 --- a/distributedcloud/dcmanager/common/utils.py +++ b/distributedcloud/dcmanager/common/utils.py @@ -35,6 +35,7 @@ from dcmanager.common import consts from dcmanager.common import exceptions from dcmanager.db import api as db_api from dcmanager.drivers.openstack import vim +from dcorch.common import consts as dcorch_consts LOG = logging.getLogger(__name__) @@ -56,9 +57,9 @@ def get_batch_projects(batch_size, project_list, fillvalue=None): def validate_quota_limits(payload): for resource in payload: # Check valid resource name - if resource not in itertools.chain(consts.CINDER_QUOTA_FIELDS, - consts.NOVA_QUOTA_FIELDS, - consts.NEUTRON_QUOTA_FIELDS): + if resource not in itertools.chain(dcorch_consts.CINDER_QUOTA_FIELDS, + dcorch_consts.NOVA_QUOTA_FIELDS, + dcorch_consts.NEUTRON_QUOTA_FIELDS): raise exceptions.InvalidInputError # Check valid quota limit value in case for put/post if isinstance(payload, dict) and (not isinstance( diff --git a/distributedcloud/dcmanager/manager/subcloud_install.py b/distributedcloud/dcmanager/manager/subcloud_install.py index 82e491dca..50a80ce50 100644 --- a/distributedcloud/dcmanager/manager/subcloud_install.py +++ b/distributedcloud/dcmanager/manager/subcloud_install.py @@ -178,7 +178,7 @@ class SubcloudInstall(object): def get_image_tag(self, image_name): tags = self.sysinv_client.get_registry_image_tags(image_name) if not tags: - msg = ("Error: Image % not found in the local registry." % + msg = ("Error: Image %s not found in the local registry." % image_name) LOG.error(msg) raise exceptions.NotFound() diff --git a/distributedcloud/dcorch/api/proxy/common/service.py b/distributedcloud/dcorch/api/proxy/common/service.py index 8a614ad8f..43f6ab7c9 100644 --- a/distributedcloud/dcorch/api/proxy/common/service.py +++ b/distributedcloud/dcorch/api/proxy/common/service.py @@ -60,7 +60,8 @@ class Middleware(Application): """ def _factory(app): - return cls(app, global_config, **local_config) + # https://bugs.launchpad.net/starlingx/+bug/1865085 + return cls(app, global_config, **local_config) # pylint: disable=too-many-function-args return _factory def __init__(self, application): diff --git a/distributedcloud/dcorch/drivers/openstack/sysinv_v1.py b/distributedcloud/dcorch/drivers/openstack/sysinv_v1.py index e977191af..cb06b72d3 100644 --- a/distributedcloud/dcorch/drivers/openstack/sysinv_v1.py +++ b/distributedcloud/dcorch/drivers/openstack/sysinv_v1.py @@ -188,7 +188,7 @@ class SysinvClient(base.DriverBase): self.region_name, trapdest_ip_address)) self.client.itrapdest.delete(trapdest_ip_address) except HTTPNotFound: - LOG.info("snmp_trapdest_delete NotFound %s for region: {}".format( + LOG.info("snmp_trapdest_delete NotFound {} for region: {}".format( trapdest_ip_address, self.region_name)) raise exceptions.TrapDestNotFound(region_name=self.region_name, ip_address=trapdest_ip_address) @@ -252,7 +252,7 @@ class SysinvClient(base.DriverBase): self.region_name, community)) self.client.icommunity.delete(community) except HTTPNotFound: - LOG.info("snmp_community_delete NotFound %s for region: {}".format( + LOG.info("snmp_community_delete NotFound {} for region: {}".format( community, self.region_name)) raise exceptions.CommunityNotFound(region_name=self.region_name, community=community) diff --git a/distributedcloud/dcorch/engine/fernet_key_manager.py b/distributedcloud/dcorch/engine/fernet_key_manager.py index bf2debda3..febc17a3a 100644 --- a/distributedcloud/dcorch/engine/fernet_key_manager.py +++ b/distributedcloud/dcorch/engine/fernet_key_manager.py @@ -111,7 +111,7 @@ class FernetKeyManager(manager.Manager): except subprocess.CalledProcessError: msg = _("Failed to rotate the keys") LOG.exception(msg) - raise exceptions.InternalError(msg) + raise exceptions.InternalError(message=msg) self._schedule_work(consts.OPERATION_TYPE_PUT) diff --git a/distributedcloud/dcorch/engine/sync_services/sysinv.py b/distributedcloud/dcorch/engine/sync_services/sysinv.py index 9acf8bda7..f59b0a8e0 100644 --- a/distributedcloud/dcorch/engine/sync_services/sysinv.py +++ b/distributedcloud/dcorch/engine/sync_services/sysinv.py @@ -852,7 +852,7 @@ class SysinvSyncThread(SyncThread): resource_type, resource.uuid)) return resource.uuid else: - LOG.info("get_resource_id {} NO uuid resource_type={}".format( + LOG.info("get_resource_id NO uuid resource_type={}".format( resource_type)) return self.RESOURCE_UUID_NULL # master_id cannot be None @@ -969,8 +969,8 @@ class SysinvSyncThread(SyncThread): # The resource differs, signal to perform the audit_action return True - LOG.info("audit_discrepancy default action".format(resource_type), - extra=self.log_extra) + LOG.info("audit_discrepancy resource_type {} default action".format( + resource_type), extra=self.log_extra) return False def audit_action(self, resource_type, finding, resource, sc_source=None): diff --git a/distributedcloud/pylint.rc b/distributedcloud/pylint.rc index 157bdbfc3..77984dba0 100644 --- a/distributedcloud/pylint.rc +++ b/distributedcloud/pylint.rc @@ -2,6 +2,11 @@ # Specify a configuration file. rcfile=pylint.rc +# A comma-separated list of package or module names from where C extensions may +# be loaded. Extensions are loading into the active Python interpreter and may +# run arbitrary code. +extension-pkg-whitelist=greenlet + # Python code to execute, usually for sys.path manipulation such as pygtk.require(). #init-hook= @@ -27,12 +32,15 @@ load-plugins= # multiple time (only on the command line, not in the configuration file where # it should appear only once). # https://pylint.readthedocs.io/en/latest/user_guide/output.html#source-code-analysis-section -# We are disabling (C)onvention -# We are disabling (R)efactor -# We are probably disabling (W)arning -# We are not disabling (F)atal, (E)rror -disable=C,R,W -#disable=C,R +# R detect Refactor for a "good practice" metric violation +# C detect Convention for coding standard violation +# W detect Warning for stylistic problems, or minor programming issues +# E detect Errors for important programming issues (i.e. most probably bug) +# E1101: no-member +# E1102: not-callable +# E1120: no-value-for-parameter (sqlalchemy) +disable=C,R,W, + E1101,E1102,E1120 [REPORTS] @@ -46,7 +54,7 @@ output-format=text files-output=no # Tells whether to display a full report or only the messages -reports=no +reports=yes # Python expression which should return a note less than 10 (10 is the highest # note). You have access to the variables errors warning, statement which @@ -83,9 +91,15 @@ indent-string=' ' # mixin class is detected if its name ends with "mixin" (case insensitive). ignore-mixin-members=yes +# List of module names for which member attributes should not be checked +# (useful for modules/projects where namespaces are manipulated during runtime +# and thus existing member attributes cannot be deduced by static analysis +ignored-modules=distutils,eventlet.green.subprocess,six,six.moves + # List of classes names for which member attributes should not be checked # (useful for classes with attributes dynamically set). -ignored-classes=SQLObject +# pylint is confused by sqlalchemy Table, as well as sqlalchemy Enum types +ignored-classes=SQLObject,Table # List of members which are set dynamically and missed by pylint inference # system, and so shouldn't trigger E0201 when accessed. Python regular diff --git a/distributedcloud/test-requirements.txt b/distributedcloud/test-requirements.txt index 551fd50fe..8c355f44a 100644 --- a/distributedcloud/test-requirements.txt +++ b/distributedcloud/test-requirements.txt @@ -19,7 +19,7 @@ oslotest>=1.10.0 # Apache-2.0 os-testr>=0.8.0 # Apache-2.0 tempest-lib>=0.14.0 # Apache-2.0 ddt>=1.0.1 # MIT -pylint==1.7.1 # GPLv2 - +pylint==1.9.2;python_version<"3.0" # GPLv2 +pylint==2.3.1;python_version>="3.0" # GPLv2 PyYAML>=3.1.0 yamllint>=0.5.2 diff --git a/distributedcloud/tox.ini b/distributedcloud/tox.ini index 4cb03782d..32db63834 100644 --- a/distributedcloud/tox.ini +++ b/distributedcloud/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = pep8,py27 +envlist = pep8,py27,pylint minversion = 2.3 skipsdist = True @@ -14,10 +14,12 @@ sysinv_src_dir = ../../config/sysinv/sysinv/sysinv tsconfig_src_dir = ../../config/tsconfig/tsconfig controllerconfig_src_dir = ../../config/controllerconfig/controllerconfig cgtsclient_src_dir = ../../config/sysinv/cgts-client/cgts-client +cgcs_patch_src_dir = ../../update/cgcs-patch/cgcs-patch [testenv] install_command = pip install \ - -c{env:UPPER_CONSTRAINTS_FILE:https://opendev.org/openstack/requirements/raw/branch/stable/stein/upper-constraints.txt} \ + -c ./upper-constraints.txt \ + -c https://opendev.org/openstack/requirements/raw/branch/stable/stein/upper-constraints.txt \ {opts} {packages} setenv = VIRTUAL_ENV={envdir} @@ -101,11 +103,11 @@ commands = coverage xml --rcfile=.coveragerc_xml coverage report -[testenv:docs] +[testenv:docs] basepython = python3 install_command = pip install -U {opts} {packages} deps = -r{toxinidir}/doc/requirements.txt -commands = +commands = rm -rf doc/build sphinx-build -a -E -W -d doc/build/doctrees -b html doc/source doc/build/html whitelist_externals = rm @@ -175,11 +177,11 @@ import_exceptions = dcmanager.common.i18n,dcorch.common.i18n [testenv:pylint] basepython = python2.7 sitepackages = False +deps = {[testenv:py27]deps} + -e{[dc]cgcs_patch_src_dir} -deps = {[testenv]deps} - -commands = - pylint {posargs} dcmanager dcorch --rcfile=./pylint.rc +commands = + pylint {posargs} dcmanager dcorch dcdbsync --rcfile=./pylint.rc [testenv:linters] basepython = python3 diff --git a/distributedcloud/upper-constraints.txt b/distributedcloud/upper-constraints.txt new file mode 100644 index 000000000..2d536f6af --- /dev/null +++ b/distributedcloud/upper-constraints.txt @@ -0,0 +1,4 @@ +# oslo.messaging locked to pike version +# https://bugs.launchpad.net/starlingx/+bug/1865054 +oslo.messaging==5.30.6 # Apache-2.0 + diff --git a/tox.ini b/tox.ini index 6f6cf142a..062ab2ac8 100644 --- a/tox.ini +++ b/tox.ini @@ -5,7 +5,7 @@ skipsdist = True [testenv] install_command = pip install \ - -c{env:UPPER_CONSTRAINTS_FILE:https://opendev.org/openstack/requirements/raw/branch/stable/stein/upper-constraints.txt} \ + -c https://opendev.org/openstack/requirements/raw/branch/stable/stein/upper-constraints.txt \ {opts} {packages} setenv = VIRTUAL_ENV={envdir}