From 542f5cc6277c6d66bb1f603e174f1254e823f2c9 Mon Sep 17 00:00:00 2001 From: Angus Lees Date: Wed, 20 Aug 2014 13:38:14 +1000 Subject: [PATCH] Add pylint tox environment and disable all existing warnings pylintrc update disables all warnings that currently trigger on neutron code. The rough plan is to slowly re-enable warning categories as we clean up code in question. This change also includes a few ultra-trivial syntax cleanups where it allowed the check to be immediately enabled for the rest of the codebase: - Added missing trailing newlines in several files (db/migration/__init__.py, nuage/{nuagedb,syncmanager,common/config}.py) - Renamed self to cls in @classmethods (cisco/db/l3/device_handling_db.py) - Removed whitespace around '=' in a kwarg (cisco/db/l3/device_handling_db.py, cisco/db/n1kv_db_v2.py) - Updated deprecated pylint 'disable-msg' directive to newer 'disable' (cisco/extensions/qos.py) - File-specific disable for too-many-format-args pending further investigation of alternatives (ml2/drivers/arista/arista_l3_driver.py) - Import module rather than object and avoid long line (services/l3_router/l3_arista.py) Change-Id: Ifb0a1a38e33f9073a78658ca578fbd2a42747724 --- .pylintrc | 125 +++++++++++++++--- .../plugins/cisco/db/l3/device_handling_db.py | 22 +-- neutron/plugins/cisco/db/n1kv_db_v2.py | 6 +- neutron/plugins/cisco/extensions/qos.py | 2 +- .../ml2/drivers/arista/arista_l3_driver.py | 2 + neutron/services/l3_router/l3_arista.py | 7 +- tox.ini | 7 + 7 files changed, 136 insertions(+), 35 deletions(-) diff --git a/.pylintrc b/.pylintrc index 87fbcd3b3df..c738c547c1d 100644 --- a/.pylintrc +++ b/.pylintrc @@ -2,17 +2,98 @@ [MASTER] # Add to the black list. It should be a base name, not a # path. You may set this option multiple times. -ignore=test +# +# Note the 'openstack' below is intended to match only +# neutron.openstack.common. If we ever have another 'openstack' +# dirname, then we'll need to expand the ignore features in pylint :/ +ignore=.git,tests,openstack -[Messages Control] -# NOTE(justinsb): We might want to have a 2nd strict pylintrc in future -# C0111: Don't require docstrings on every method -# W0511: TODOs in code comments are fine. -# W0142: *args and **kwargs are fine. -# W0622: Redefining id is fine. -disable=C0111,W0511,W0142,W0622 +[MESSAGES CONTROL] +# NOTE(gus): This is a long list. A number of these are important and +# should be re-enabled once the offending code is fixed (or marked +# with a local disable) +disable= +# "F" Fatal errors that prevent further processing + import-error, +# "I" Informational noise + locally-disabled, +# "E" Error for important programming issues (likely bugs) + access-member-before-definition, + assignment-from-no-return, + bad-except-order, + bad-super-call, + maybe-no-member, + no-member, + no-method-argument, + no-name-in-module, + no-self-argument, + not-callable, + no-value-for-parameter, + super-on-old-class, + too-few-format-args, +# "W" Warnings for stylistic problems or minor programming issues + abstract-method, + anomalous-backslash-in-string, + anomalous-unicode-escape-in-string, + arguments-differ, + attribute-defined-outside-init, + bad-builtin, + bad-indentation, + broad-except, + dangerous-default-value, + deprecated-lambda, + duplicate-key, + expression-not-assigned, + fixme, + global-statement, + global-variable-not-assigned, + logging-not-lazy, + lost-exception, + no-init, + non-parent-init-called, + pointless-string-statement, + protected-access, + redefined-builtin, + redefined-outer-name, + redefine-in-handler, + reimported, + signature-differs, + star-args, + super-init-not-called, + undefined-loop-variable, + unnecessary-lambda, + unnecessary-pass, + unpacking-non-sequence, + unreachable, + unused-argument, + unused-import, + unused-variable, + useless-else-on-loop, +# "C" Coding convention violations + bad-continuation, + invalid-name, + missing-docstring, + old-style-class, + superfluous-parens, +# "R" Refactor recommendations + abstract-class-little-used, + abstract-class-not-used, + cyclic-import, + duplicate-code, + interface-not-implemented, + no-self-use, + 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-public-methods, + too-many-return-statements, + too-many-statements -[Basic] +[BASIC] # Variable names can be 1 to 31 characters long, with lowercase and underscores variable-rgx=[a-z_][a-z0-9_]{0,30}$ @@ -21,7 +102,7 @@ argument-rgx=[a-z_][a-z0-9_]{1,30}$ # Method names should be at least 3 characters long # and be lowecased with underscores -method-rgx=([a-z_][a-z0-9_]{2,50}|setUp|tearDown)$ +method-rgx=([a-z_][a-z0-9_]{2,}|setUp|tearDown)$ # Module names matching neutron-* are ok (files in bin/) module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+)|(neutron-[a-z0-9_-]+))$ @@ -29,14 +110,26 @@ module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+)|(neutron-[a-z0-9_-]+))$ # Don't require docstrings on tests. no-docstring-rgx=((__.*__)|([tT]est.*)|setUp|tearDown)$ -[Design] -max-public-methods=100 -min-public-methods=0 -max-args=6 - -[Variables] +[FORMAT] +# Maximum number of characters on a single line. +max-line-length=79 +[VARIABLES] # List of additional names supposed to be defined in builtins. Remember that # you should avoid to define new builtins when possible. # _ is used by our localization additional-builtins=_ + +[CLASSES] +# List of interface methods to ignore, separated by a comma. +ignore-iface-methods= + +[IMPORTS] +# Deprecated modules which should not be used, separated by a comma +deprecated-modules= +# should use openstack.common.jsonutils + json + +[REPORTS] +# Tells whether to display a full report or only the messages +reports=no diff --git a/neutron/plugins/cisco/db/l3/device_handling_db.py b/neutron/plugins/cisco/db/l3/device_handling_db.py index a2834899182..4e5deccd709 100644 --- a/neutron/plugins/cisco/db/l3/device_handling_db.py +++ b/neutron/plugins/cisco/db/l3/device_handling_db.py @@ -187,30 +187,30 @@ class DeviceHandlingMixin(object): return cls._mgmt_sec_grp_id @classmethod - def get_hosting_device_driver(self): + def get_hosting_device_driver(cls): """Returns device driver.""" - if self._hosting_device_driver: - return self._hosting_device_driver + if cls._hosting_device_driver: + return cls._hosting_device_driver else: try: - self._hosting_device_driver = importutils.import_object( + cls._hosting_device_driver = importutils.import_object( cfg.CONF.hosting_devices.csr1kv_device_driver) except (ImportError, TypeError, n_exc.NeutronException): LOG.exception(_('Error loading hosting device driver')) - return self._hosting_device_driver + return cls._hosting_device_driver @classmethod - def get_hosting_device_plugging_driver(self): + def get_hosting_device_plugging_driver(cls): """Returns plugging driver.""" - if self._plugging_driver: - return self._plugging_driver + if cls._plugging_driver: + return cls._plugging_driver else: try: - self._plugging_driver = importutils.import_object( + cls._plugging_driver = importutils.import_object( cfg.CONF.hosting_devices.csr1kv_plugging_driver) except (ImportError, TypeError, n_exc.NeutronException): LOG.exception(_('Error loading plugging driver')) - return self._plugging_driver + return cls._plugging_driver def get_hosting_devices_qry(self, context, hosting_device_ids, load_agent=True): @@ -432,7 +432,7 @@ class DeviceHandlingMixin(object): with context.session.begin(subtransactions=True): hd_db = l3_models.HostingDevice( id=hd.get('id') or uuidutils.generate_uuid(), - complementary_id = hd.get('complementary_id'), + complementary_id=hd.get('complementary_id'), tenant_id=tenant_id, device_id=hd.get('device_id'), admin_state_up=hd.get('admin_state_up', True), diff --git a/neutron/plugins/cisco/db/n1kv_db_v2.py b/neutron/plugins/cisco/db/n1kv_db_v2.py index d694b236764..20c9706f0aa 100644 --- a/neutron/plugins/cisco/db/n1kv_db_v2.py +++ b/neutron/plugins/cisco/db/n1kv_db_v2.py @@ -943,9 +943,9 @@ def update_profile_binding(db_session, profile_id, tenants, profile_type): profile_id=profile_id, profile_type=profile_type).delete() new_tenants_set = set(tenants) for tenant_id in new_tenants_set: - tenant = n1kv_models_v2.ProfileBinding(profile_type = profile_type, - tenant_id = tenant_id, - profile_id = profile_id) + tenant = n1kv_models_v2.ProfileBinding(profile_type=profile_type, + tenant_id=tenant_id, + profile_id=profile_id) db_session.add(tenant) diff --git a/neutron/plugins/cisco/extensions/qos.py b/neutron/plugins/cisco/extensions/qos.py index 5d6cb7b6073..db642f1d4b7 100644 --- a/neutron/plugins/cisco/extensions/qos.py +++ b/neutron/plugins/cisco/extensions/qos.py @@ -94,7 +94,7 @@ class QosController(common.NeutronController, wsgi.Controller): result = [builder.build(qos, is_detail)['qos'] for qos in qoss] return dict(qoss=result) - # pylint: disable-msg=E1101 + # pylint: disable=no-member def show(self, request, tenant_id, id): """Returns qos details for the given qos id.""" try: diff --git a/neutron/plugins/ml2/drivers/arista/arista_l3_driver.py b/neutron/plugins/ml2/drivers/arista/arista_l3_driver.py index aac24410f02..5aa64d8a380 100644 --- a/neutron/plugins/ml2/drivers/arista/arista_l3_driver.py +++ b/neutron/plugins/ml2/drivers/arista/arista_l3_driver.py @@ -33,6 +33,8 @@ VIRTUAL_ROUTER_MAC = '00:11:22:33:44:55' IPV4_BITS = 32 IPV6_BITS = 128 +# This string-format-at-a-distance confuses pylint :( +# pylint: disable=too-many-format-args router_in_vrf = { 'router': {'create': ['vrf definition {0}', 'rd {1}', diff --git a/neutron/services/l3_router/l3_arista.py b/neutron/services/l3_router/l3_arista.py index 2c6cf9cc701..89fc98b37e9 100644 --- a/neutron/services/l3_router/l3_arista.py +++ b/neutron/services/l3_router/l3_arista.py @@ -32,8 +32,7 @@ from neutron.openstack.common import excutils from neutron.openstack.common import log as logging from neutron.plugins.common import constants from neutron.plugins.ml2.driver_context import NetworkContext # noqa -from neutron.plugins.ml2.drivers.arista.arista_l3_driver import AristaL3Driver # noqa -from neutron.plugins.ml2.drivers.arista.arista_l3_driver import NeutronNets # noqa +from neutron.plugins.ml2.drivers.arista import arista_l3_driver LOG = logging.getLogger(__name__) @@ -54,8 +53,8 @@ class AristaL3ServicePlugin(db_base_plugin_v2.NeutronDbPluginV2, def __init__(self, driver=None): - self.driver = driver or AristaL3Driver() - self.ndb = NeutronNets() + self.driver = driver or arista_l3_driver.AristaL3Driver() + self.ndb = arista_l3_driver.NeutronNets() self.setup_rpc() self.sync_timeout = cfg.CONF.l3_arista.l3_sync_interval self.sync_lock = threading.Lock() diff --git a/tox.ini b/tox.ini index 90cc8d766cf..2c257833f27 100644 --- a/tox.ini +++ b/tox.ini @@ -84,6 +84,13 @@ show-source = true builtins = _ exclude = .venv,.git,.tox,dist,doc,*openstack/common*,*lib/python*,*egg,build,tools,.ropeproject,rally-scenarios +[testenv:pylint] +deps = + {[testenv]deps} + pylint +commands = + pylint --rcfile=.pylintrc --output-format=colorized {posargs:neutron} + [hacking] import_exceptions = neutron.openstack.common.gettextutils local-check-factory = neutron.hacking.checks.factory