[pylint] Fix/ignore pylint errors in non-test modules

Pylint does not play very well with dynamic object
manipulation in python This creates a lot of
false-positives in the code-base which affects
contributors looking for genuine failures.

So, this change
- adds pylint ignore statements where appropriate
  to disable testing these lines of code and failing.
- replaces all the pylint error codes (they are
  hard to remember/relate to) with error names
  which are easier to understand when reading the
  code.
- initializes sqlalchemy model objects as dictionaries
  which is a valid representation over None.
- removes ignore directives on six.moves which
  is globally ignored in our pylintrc.
- adds alembic.op to the ignored
  modules list since they are not supported by
  pylint and have known issues.

This patch is the beginning of a series of
commits to use pylint in a sane way on manila code.

Change-Id: I44616821c5311d6f14986697efbbe5624de364a5
This commit is contained in:
Goutham Pacha Ravi 2019-02-18 17:36:38 -08:00
parent 1701821b89
commit 3e855d5f60
48 changed files with 92 additions and 49 deletions

View File

@ -185,5 +185,4 @@ additional-builtins=_
[TYPECHECK]
# List of module names for which member attributes should not be checked
ignored-modules=six.moves,_MovedItems
ignored-modules=six.moves,_MovedItems,alembic.op

View File

@ -73,6 +73,7 @@ class APIRouter(base_wsgi.Router):
def __init__(self, ext_mgr=None):
if ext_mgr is None:
if self.ExtensionManager:
# pylint: disable=not-callable
ext_mgr = self.ExtensionManager()
else:
raise Exception(_("Must specify an ExtensionManager class"))

View File

@ -1023,6 +1023,7 @@ class Controller(object):
if view_builder:
self._view_builder = view_builder
elif self._view_builder_class:
# pylint: disable=not-callable
self._view_builder = self._view_builder_class()
else:
self._view_builder = None

View File

@ -39,7 +39,7 @@ class SchedulerStatsController(wsgi.Controller):
@wsgi.Controller.api_version('2.23') # noqa
@wsgi.Controller.authorize('index')
def pools_index(self, req): # pylint: disable=E0102
def pools_index(self, req): # pylint: disable=function-redefined
return self._pools(req, action='index', enable_share_type=True)
@wsgi.Controller.api_version('1.0', '2.22')
@ -50,7 +50,7 @@ class SchedulerStatsController(wsgi.Controller):
@wsgi.Controller.api_version('2.23') # noqa
@wsgi.Controller.authorize('detail')
def pools_detail(self, req): # pylint: disable=E0102
def pools_detail(self, req): # pylint: disable=function-redefined
return self._pools(req, action='detail', enable_share_type=True)
def _pools(self, req, action='index', enable_share_type=False):

View File

@ -153,7 +153,7 @@ class ShareTypeExtraSpecsController(wsgi.Controller):
@wsgi.Controller.api_version('2.24') # noqa
@wsgi.Controller.authorize
def delete(self, req, type_id, id): # pylint: disable=E0102
def delete(self, req, type_id, id): # pylint: disable=function-redefined
"""Deletes an existing extra spec."""
context = req.environ['manila.context']
self._check_type(context, type_id)

View File

@ -74,7 +74,7 @@ class ShareExportLocationController(wsgi.Controller):
return self._index(req, share_id)
@wsgi.Controller.api_version('2.47') # noqa: F811
def index(self, req, share_id): # pylint: disable=E0102
def index(self, req, share_id): # pylint: disable=function-redefined
"""Return a list of export locations for share."""
return self._index(req, share_id,
ignore_secondary_replicas=True)
@ -85,7 +85,7 @@ class ShareExportLocationController(wsgi.Controller):
return self._show(req, share_id, export_location_uuid)
@wsgi.Controller.api_version('2.47') # noqa: F811
def show(self, req, share_id, # pylint: disable=E0102
def show(self, req, share_id, # pylint: disable=function-redefined
export_location_uuid):
"""Return data about the requested export location."""
return self._show(req, share_id, export_location_uuid,

View File

@ -64,7 +64,7 @@ class ShareInstancesController(wsgi.Controller, wsgi.AdminActionsMixin):
@wsgi.Controller.api_version("2.3", "2.34") # noqa
@wsgi.Controller.authorize
def index(self, req): # pylint: disable=E0102
def index(self, req): # pylint: disable=function-redefined
context = req.environ['manila.context']
req.GET.pop('export_location_id', None)
@ -74,7 +74,7 @@ class ShareInstancesController(wsgi.Controller, wsgi.AdminActionsMixin):
@wsgi.Controller.api_version("2.35") # noqa
@wsgi.Controller.authorize
def index(self, req): # pylint: disable=E0102
def index(self, req): # pylint: disable=function-redefined
context = req.environ['manila.context']
filters = {}
filters.update(req.GET)

View File

@ -47,7 +47,7 @@ class ShareTypesController(wsgi.Controller):
def __getattr__(self, key):
if key == 'os-share-type-access':
return self.share_type_access
return super(ShareTypesController, self).__getattr__(key)
return super(ShareTypesController, self).__getattribute__(key)
def _notify_share_type_error(self, context, method, payload):
rpc.get_notifier('shareType').error(context, method, payload)
@ -171,7 +171,7 @@ class ShareTypesController(wsgi.Controller):
@wsgi.Controller.api_version("2.24") # noqa
@wsgi.action("create")
def create(self, req, body): # pylint: disable=E0102
def create(self, req, body): # pylint: disable=function-redefined
return self._create(req, body, set_defaults=False)
@wsgi.Controller.authorize('create')

View File

@ -185,18 +185,18 @@ class ShareController(shares.ShareMixin,
check_availability_zones_extra_spec=True)
@wsgi.Controller.api_version("2.31", "2.47") # noqa
def create(self, req, body): # pylint: disable=E0102
def create(self, req, body): # pylint: disable=function-redefined
return self._create(
req, body, check_create_share_from_snapshot_support=True)
@wsgi.Controller.api_version("2.24", "2.30") # noqa
def create(self, req, body): # pylint: disable=E0102
def create(self, req, body): # pylint: disable=function-redefined
body.get('share', {}).pop('share_group_id', None)
return self._create(req, body,
check_create_share_from_snapshot_support=True)
@wsgi.Controller.api_version("2.0", "2.23") # noqa
def create(self, req, body): # pylint: disable=E0102
def create(self, req, body): # pylint: disable=function-redefined
body.get('share', {}).pop('share_group_id', None)
return self._create(req, body)
@ -411,7 +411,7 @@ class ShareController(shares.ShareMixin,
return detail
@wsgi.Controller.api_version("2.8") # noqa
def manage(self, req, body): # pylint: disable=E0102
def manage(self, req, body): # pylint: disable=function-redefined
detail = self._manage(req, body)
return detail

View File

@ -86,7 +86,7 @@ class VersionsController(wsgi.Controller):
return builder.build_versions(known_versions)
@wsgi.Controller.api_version('2.0') # noqa
def index(self, req): # pylint: disable=E0102
def index(self, req): # pylint: disable=function-redefined
"""Return versions supported after the start of microversions."""
builder = views_versions.get_view_builder(req)
known_versions = copy.deepcopy(_KNOWN_VERSIONS)

View File

@ -29,11 +29,12 @@ def run_migrations_online():
engine = db_api.get_engine()
connection = engine.connect()
target_metadata = db_models.ManilaBase.metadata
context.configure(connection=connection, # pylint: disable=E1101
# pylint: disable=no-member
context.configure(connection=connection,
target_metadata=target_metadata)
try:
with context.begin_transaction(): # pylint: disable=E1101
context.run_migrations() # pylint: disable=E1101
with context.begin_transaction():
context.run_migrations()
finally:
connection.close()

View File

@ -16,7 +16,7 @@ import os
import alembic
from alembic import config as alembic_config
import alembic.migration as alembic_migration
import alembic.migration as alembic_migration # pylint: disable=import-error
from oslo_config import cfg
from manila.db.sqlalchemy import api as db_api

View File

@ -210,6 +210,7 @@ def downgrade():
si_table = utils.load_table('share_instances', connection)
member_table = utils.load_table('cgsnapshot_members', connection)
for si_record in connection.execute(si_table.select()):
# pylint: disable=no-value-for-parameter
connection.execute(
member_table.update().where(
member_table.c.share_instance_id == si_record.id,

View File

@ -81,6 +81,7 @@ def upgrade():
connection, services_table, availability_zones_table)
# Map string AZ names to ID's in target tables
# pylint: disable=no-value-for-parameter
set_az_id_in_table = lambda table, id, name: (
op.execute(
table.update().where(table.c.availability_zone == name).values(
@ -112,6 +113,7 @@ def downgrade():
services_table = utils.load_table('services', connection)
for az in connection.execute(az_table.select()):
# pylint: disable=no-value-for-parameter
op.execute(
share_instances_table.update().where(
share_instances_table.c.availability_zone_id == az.id

View File

@ -84,6 +84,7 @@ def upgrade():
if priorities[access_rule['state']] > priorities[status]:
status = access_rule['state']
# pylint: disable=no-value-for-parameter
op.execute(
share_instances_table.update().where(
share_instances_table.c.id == instance['id']
@ -119,6 +120,7 @@ def downgrade():
else:
state = 'error'
# pylint: disable=no-value-for-parameter
op.execute(
instance_access_table.update().where(
instance_access_table.c.share_instance_id == instance['id']

View File

@ -51,7 +51,7 @@ def _transform_case(table_name, make_upper):
for row in connection.execute(table.select()):
op.execute(
table.update().where(
table.update().where( # pylint: disable=no-value-for-parameter
table.c.id == row.id
).values({'status': case(row.status)})
)

View File

@ -116,6 +116,7 @@ def upgrade():
sa.Column('snapshot_support', sa.Boolean),
sa.Column('create_share_from_snapshot_support', sa.Boolean),
)
# pylint: disable=no-value-for-parameter
update = shares_table.update().where(
shares_table.c.deleted == 'False').values(
create_share_from_snapshot_support=shares_table.c.snapshot_support)
@ -139,6 +140,7 @@ def downgrade():
autoload=True,
autoload_with=connection)
# pylint: disable=no-value-for-parameter
update = extra_specs.update().where(
extra_specs.c.spec_key ==
constants.ExtraSpecs.CREATE_SHARE_FROM_SNAPSHOT_SUPPORT).where(

View File

@ -49,6 +49,7 @@ def upgrade():
for instance in connection.execute(share_instances_table.select()):
share = connection.execute(shares_table.select().where(
instance['share_id'] == shares_table.c.id)).first()
# pylint: disable=no-value-for-parameter
op.execute(share_instances_table.update().where(
share_instances_table.c.id == instance['id']).values(
{'share_type_id': share['share_type_id']}))
@ -75,6 +76,7 @@ def downgrade():
for share in connection.execute(shares_table.select()):
instance = connection.execute(share_instances_table.select().where(
share['id'] == share_instances_table.c.share_id)).first()
# pylint: disable=no-value-for-parameter
op.execute(shares_table.update().where(
shares_table.c.id == instance['share_id']).values(
{'share_type_id': instance['share_type_id']}))

View File

@ -120,6 +120,7 @@ def remove_share_instances_table(connection):
share_inst_table.c.share_id == share.id)
).first()
# pylint: disable=no-value-for-parameter
op.execute(
shares_table.update().where(
shares_table.c.id == share.id
@ -204,6 +205,7 @@ def remove_snapshot_instances_table(connection):
snapshots_table.c.id == snapshot_instance.snapshot_id)
).first()
# pylint: disable=no-value-for-parameter
op.execute(
snapshots_table.update().where(
snapshots_table.c.id == snapshot.id
@ -235,6 +237,7 @@ def upgrade_export_locations_table(connection):
share_instances_table.c.share_id == export.share_id)
).first()
# pylint: disable=no-value-for-parameter
op.execute(
share_el_table.update().where(
share_el_table.c.id == export.id
@ -265,6 +268,7 @@ def downgrade_export_locations_table(connection):
share_instances_table.c.id == export.share_instance_id)
).first()
# pylint: disable=no-value-for-parameter
op.execute(
share_el_table.update().where(
share_el_table.c.id == export.id

View File

@ -67,6 +67,7 @@ def upgrade():
for instance in connection.execute(instances_query):
access_rule_status = instance['access_rules_status']
# pylint: disable=no-value-for-parameter
op.execute(
instance_access_map_table.update().where(
instance_access_map_table.c.share_instance_id == instance['id']
@ -91,6 +92,7 @@ def downgrade():
connection = op.get_bind()
share_instances_table = utils.load_table('share_instances', connection)
# pylint: disable=no-value-for-parameter
op.execute(
share_instances_table.update().where(
share_instances_table.c.access_rules_status ==

View File

@ -95,6 +95,7 @@ def upgrade():
autoload=True,
autoload_with=connection)
# pylint: disable=no-value-for-parameter
update = shares.update().where(shares.c.deleted == 'False').values(
snapshot_support=True)
connection.execute(update)
@ -113,6 +114,7 @@ def downgrade():
autoload=True,
autoload_with=connection)
# pylint: disable=no-value-for-parameter
update = extra_specs.update().where(
extra_specs.c.spec_key == constants.ExtraSpecs.SNAPSHOT_SUPPORT).where(
extra_specs.c.deleted == 0).values(

View File

@ -103,6 +103,7 @@ def downgrade():
autoload=True, autoload_with=connection)
for location in export_locations:
# pylint: disable=no-value-for-parameter
update = (shares.update().where(shares.c.id == location.share_id).
values(export_location=location.path))
connection.execute(update)

View File

@ -55,6 +55,7 @@ def upgrade():
instances_table = utils.load_table('share_instances', connection)
for access_rule in connection.execute(access_table.select()):
# pylint: disable=assignment-from-no-return
instances_query = instances_table.select().where(
instances_table.c.share_id == access_rule.share_id
)
@ -98,6 +99,7 @@ def downgrade():
instance_access_table.c.access_id == access_rule['id'])
).first()
# pylint: disable=no-value-for-parameter
op.execute(
access_table.update().where(
access_table.c.id == access_rule['id']

View File

@ -48,6 +48,7 @@ def upgrade():
sa.Column('deleted', sa.String(length=36)),
sa.Column('revert_to_snapshot_support', sa.Boolean),
)
# pylint: disable=no-value-for-parameter
update = shares_table.update().where(
shares_table.c.deleted == 'False').values(
revert_to_snapshot_support=False)

View File

@ -86,6 +86,7 @@ def upgrade():
connection = op.get_bind()
shares_table = utils.load_table('shares', connection)
# pylint: disable=no-value-for-parameter
op.execute(
shares_table.update().where(
shares_table.c.deleted == 'False').values({

View File

@ -140,7 +140,7 @@ def downgrade():
# Remove copied records from source table
connection.execute(
ssi_table.delete().where(
ssi_table.delete().where( # pylint: disable=no-value-for-parameter
ssi_table.c.share_group_snapshot_id.isnot(None)))
# Remove redundant fields from 'share_snapshot_instance' table

View File

@ -63,6 +63,7 @@ def upgrade():
sa.Column('is_admin_only', sa.Boolean),
)
for record in el_table.select().execute():
# pylint: disable=no-value-for-parameter
el_table.update().values(
is_admin_only=False,
uuid=uuidutils.generate_uuid(),

View File

@ -46,6 +46,7 @@ def upgrade():
# First, set the value of ``cast_rules_to_readonly`` in every existing
# share instance to False
# pylint: disable=no-value-for-parameter
op.execute(
share_instances_table.update().values({
'cast_rules_to_readonly': False,
@ -78,6 +79,7 @@ def upgrade():
)
)
for replica in connection.execute(secondary_replicas_query):
# pylint: disable=no-value-for-parameter
op.execute(
share_instances_table.update().where(
share_instances_table.c.id == replica.id

View File

@ -39,6 +39,7 @@ def upgrade():
op.add_column('share_types', is_public)
share_types = sql.Table('share_types', meta, is_public.copy())
# pylint: disable=no-value-for-parameter
share_types.update().values(is_public=True).execute()
except Exception:
LOG.error("Column |%s| not created!", repr(is_public))

View File

@ -1861,8 +1861,9 @@ def _share_get_all_with_filters(context, project_id=None, share_server_id=None,
if 'metadata' in filters:
for k, v in filters['metadata'].items():
# pylint: disable=no-member
query = query.filter(
or_(models.Share.share_metadata.any( # pylint: disable=E1101
or_(models.Share.share_metadata.any(
key=k, value=v)))
if 'extra_specs' in filters:
query = query.join(

View File

@ -431,7 +431,7 @@ class ShareInstanceExportLocations(BASE, ManilaBase):
@property
def el_metadata(self):
el_metadata = {}
for meta in self._el_metadata_bare: # pylint: disable=E1101
for meta in self._el_metadata_bare: # pylint: disable=no-member
el_metadata[meta['key']] = meta['value']
return el_metadata
@ -472,7 +472,7 @@ class ShareInstanceExportLocationsMetadata(BASE, ManilaBase):
@property
def export_location_uuid(self):
return self.export_location.uuid # pylint: disable=E1101
return self.export_location.uuid # pylint: disable=no-member
class ShareTypes(BASE, ManilaBase):

View File

@ -294,10 +294,12 @@ class ShareDriver(object):
if hasattr(self, 'init_execute_mixin'):
# Instance with 'ExecuteMixin'
self.init_execute_mixin(*args, **kwargs) # pylint: disable=E1101
# pylint: disable=no-member
self.init_execute_mixin(*args, **kwargs)
if hasattr(self, 'init_ganesha_mixin'):
# Instance with 'GaneshaMixin'
self.init_ganesha_mixin(*args, **kwargs) # pylint: disable=E1101
# pylint: disable=no-member
self.init_ganesha_mixin(*args, **kwargs)
@property
def admin_network_api(self):
@ -2370,7 +2372,9 @@ class ShareDriver(object):
if not ret_function:
ret_function = CONF.filter_function
if not ret_function:
# pylint: disable=assignment-from-none
ret_function = self.get_default_filter_function()
# pylint: enable=assignment-from-none
return ret_function
def get_goodness_function(self):
@ -2387,7 +2391,9 @@ class ShareDriver(object):
if not ret_function:
ret_function = CONF.goodness_function
if not ret_function:
# pylint: disable=assignment-from-none
ret_function = self.get_default_goodness_function()
# pylint: enable=assignment-from-none
return ret_function
def get_default_filter_function(self):

View File

@ -20,8 +20,8 @@ from oslo_log import log
from oslo_utils import excutils
import six
from six.moves import http_cookiejar
from six.moves.urllib import error as url_error # pylint: disable=E0611
from six.moves.urllib import request as url_request # pylint: disable=E0611
from six.moves.urllib import error as url_error
from six.moves.urllib import request as url_request
from manila import exception
from manila.i18n import _

View File

@ -47,7 +47,7 @@ class StorageObjectManager(object):
elt_maker = builder.ElementMaker(nsmap={None: constants.XML_NAMESPACE})
xml_parser = parser.XMLAPIParser()
obj_types = StorageObject.__subclasses__() # pylint: disable=E1101
obj_types = StorageObject.__subclasses__() # pylint: disable=no-member
for item in obj_types:
key = item.__name__
self.context[key] = eval(key)(self.connectors,

View File

@ -47,7 +47,7 @@ class StorageObjectManager(object):
elt_maker = builder.ElementMaker(nsmap={None: constants.XML_NAMESPACE})
xml_parser = parser.XMLAPIParser()
obj_types = StorageObject.__subclasses__() # pylint: disable=E1101
obj_types = StorageObject.__subclasses__() # pylint: disable=no-member
for item in obj_types:
key = item.__name__
self.context[key] = eval(key)(self.connectors,

View File

@ -22,7 +22,7 @@ from manila import utils
# Suppress the Insecure request warnings
requests.packages.urllib3.disable_warnings()
requests.packages.urllib3.disable_warnings() # pylint: disable=no-member
class HSPRestBackend(object):

View File

@ -30,7 +30,7 @@ from manila import utils
hpe3parclient = importutils.try_import("hpe3parclient")
if hpe3parclient:
from hpe3parclient import file_client
from hpe3parclient import file_client # pylint: disable=import-error
LOG = log.getLogger(__name__)

View File

@ -39,10 +39,12 @@ class RestHelper(object):
self.url = None
self.session = None
# pylint: disable=no-member
requests.packages.urllib3.disable_warnings(
requests.packages.urllib3.exceptions.InsecureRequestWarning)
requests.packages.urllib3.disable_warnings(
requests.packages.urllib3.exceptions.InsecurePlatformWarning)
# pylint: enable=no-member
def init_http_head(self):
self.url = None

View File

@ -170,10 +170,13 @@ class GPFSShareDriver(driver.ExecuteMixin, driver.GaneshaMixin,
return self._run_ssh(host, cmd, ignore_exit_code, check_exit_code)
def _sanitize_command(self, cmd_list):
# pylint: disable=too-many-function-args
return ' '.join(six.moves.shlex_quote(cmd_arg) for cmd_arg in cmd_list)
def _run_ssh(self, host, cmd_list, ignore_exit_code=None,
check_exit_code=True):
command = ' '.join(six.moves.shlex_quote(cmd_arg)
for cmd_arg in cmd_list)
command = self._sanitize_command(cmd_list)
if not self.sshpool:
gpfs_ssh_login = self.configuration.gpfs_ssh_login
password = self.configuration.gpfs_ssh_password

View File

@ -21,20 +21,23 @@
import base64
import json
import requests
from requests.packages.urllib3 import exceptions
import time
from oslo_log import log
from oslo_serialization import jsonutils
import requests
# pylint: disable=no-member,import-error
from requests.packages.urllib3 import exceptions
requests.packages.urllib3.disable_warnings(exceptions.InsecureRequestWarning)
requests.packages.urllib3.disable_warnings(
exceptions.InsecurePlatformWarning)
# pylint: enable=no-member,import-error
from manila import exception
from manila.i18n import _
LOG = log.getLogger(__name__)
requests.packages.urllib3.disable_warnings(exceptions.InsecureRequestWarning)
requests.packages.urllib3.disable_warnings(
exceptions.InsecurePlatformWarning)
session = requests.Session()

View File

@ -162,7 +162,7 @@ class NASHelperBase(object):
:return: share helper instance.
"""
self.configuration = configuration
self.init_execute_mixin() # pylint: disable=E1101
self.init_execute_mixin() # pylint: disable=no-member
self.verify_setup()
@abc.abstractmethod

View File

@ -23,7 +23,6 @@ import time
from oslo_serialization import jsonutils
import six
from six.moves import http_client
# pylint: disable=E0611,F0401
from six.moves.urllib import error as urlerror
from six.moves.urllib import request as urlrequest

View File

@ -210,7 +210,7 @@ class API(base.Base):
if original_share_group:
options['host'] = original_share_group['host']
share_group = None
share_group = {}
try:
share_group = self.db.share_group_create(context, options)
if share_group_snapshot:
@ -395,7 +395,7 @@ class API(base.Base):
})
raise exception.ShareGroupSnapshotsLimitExceeded()
snap = None
snap = {}
try:
snap = self.db.share_group_snapshot_create(context, options)
members = []

View File

@ -89,7 +89,7 @@ class Database(fixtures.Fixture):
if self.sql_connection == "sqlite://":
conn = self.engine.connect()
conn.connection.executescript(self._DB)
self.addCleanup(self.engine.dispose) # pylint: disable=E1101
self.addCleanup(self.engine.dispose) # pylint: disable=no-member
else:
shutil.copyfile(
os.path.join(CONF.state_path, self.sqlite_clean_db),

View File

@ -269,7 +269,7 @@ class ExperimentalAPITestCase(test.TestCase):
return {'fake_key': 'fake_value'}
@wsgi.Controller.api_version('2.1', '2.1', experimental=True) # noqa
def index(self, req): # pylint: disable=E0102
def index(self, req): # pylint: disable=function-redefined
return {'fake_key': 'fake_value'}
def setUp(self):

View File

@ -16,8 +16,8 @@
from eventlet import greenthread
import mock
from oslo_concurrency import processutils
from six.moves.urllib import error as url_error # pylint: disable=E0611
from six.moves.urllib import request as url_request # pylint: disable=E0611
from six.moves.urllib import error as url_error
from six.moves.urllib import request as url_request
from manila import exception
from manila.share import configuration as conf

View File

@ -114,7 +114,7 @@ class SSHPool(pools.Pool):
self.path_to_private_key = privatekey
super(SSHPool, self).__init__(*args, **kwargs)
def create(self):
def create(self): # pylint: disable=method-hidden
ssh = paramiko.SSHClient()
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
look_for_keys = True

View File

@ -148,6 +148,7 @@ class Middleware(Application):
@webob.dec.wsgify(RequestClass=Request)
def __call__(self, req):
# pylint: disable=assignment-from-none
response = self.process_request(req)
if response:
return response