From f18a33b32249f2a340f4fda95234f2e9b88076fd Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Thu, 28 Jan 2021 15:33:22 -0500 Subject: [PATCH] Introduce mypy This adds the "tox -e mypy" environment which works the same way we have introduced mypy in Cinder. Files added to mypy-files.txt are validated with mypy. Change-Id: I6d09422dbdf5ea58661aad7a63c4d4d7a2839833 --- lower-constraints.txt | 1 + mypy-files.txt | 8 +++ os_brick/encryptors/luks.py | 2 +- os_brick/executor.py | 3 +- os_brick/initiator/connectors/base.py | 12 ++-- os_brick/initiator/connectors/iscsi.py | 89 +++++++++++++++----------- os_brick/initiator/linuxscsi.py | 26 +++++--- os_brick/remotefs/remotefs.py | 14 ++-- setup.cfg | 12 ++++ test-requirements.txt | 1 + tools/mypywrap.sh | 24 +++++++ tox.ini | 6 ++ 12 files changed, 136 insertions(+), 62 deletions(-) create mode 100644 mypy-files.txt create mode 100644 tools/mypywrap.sh diff --git a/lower-constraints.txt b/lower-constraints.txt index e39743e39..29a08acaf 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -24,6 +24,7 @@ keystoneauth1==4.2.1 linecache2==1.0.0 lxml==4.5.2 msgpack==1.0.0 +mypy==0.782 netaddr==0.8.0 netifaces==0.10.9 os-client-config==1.28.0 diff --git a/mypy-files.txt b/mypy-files.txt new file mode 100644 index 000000000..16a6c7168 --- /dev/null +++ b/mypy-files.txt @@ -0,0 +1,8 @@ +os_brick/executor.py +os_brick/initiator/linuxscsi.py +os_brick/initiator/connectors/base.py +os_brick/initiator/connectors/base_iscsi.py +os_brick/initiator/connectors/iscsi.py +os_brick/remotefs/remotefs.py +os_brick/initiator/linuxrbd.py +os_brick/encryptors/luks.py diff --git a/os_brick/encryptors/luks.py b/os_brick/encryptors/luks.py index 97e88dea5..caca22d3c 100644 --- a/os_brick/encryptors/luks.py +++ b/os_brick/encryptors/luks.py @@ -224,7 +224,7 @@ class Luks2Encryptor(LuksEncryptor): connection_info=connection_info, keymgr=keymgr, execute=execute, - *args, **kwargs) + *args, **kwargs) # type: ignore def _format_volume(self, passphrase, **kwargs): """Creates a LUKS v2 header on the volume. diff --git a/os_brick/executor.py b/os_brick/executor.py index a1afdc260..ecfdd8f56 100644 --- a/os_brick/executor.py +++ b/os_brick/executor.py @@ -19,6 +19,7 @@ """ import threading +from typing import Tuple from oslo_concurrency import processutils as putils from oslo_context import context as context_utils @@ -47,7 +48,7 @@ class Executor(object): if value: setattr(exc, field, cls.safe_decode(value)) - def _execute(self, *args, **kwargs): + def _execute(self, *args, **kwargs) -> Tuple[str, str]: try: result = self.__execute(*args, **kwargs) if result: diff --git a/os_brick/initiator/connectors/base.py b/os_brick/initiator/connectors/base.py index e749eb096..c9d13fb73 100644 --- a/os_brick/initiator/connectors/base.py +++ b/os_brick/initiator/connectors/base.py @@ -31,7 +31,7 @@ LOG = logging.getLogger(__name__) class BaseLinuxConnector(initiator_connector.InitiatorConnector): os_type = initiator.OS_TYPE_LINUX - def __init__(self, root_helper, driver=None, execute=None, + def __init__(self, root_helper: str, driver=None, execute=None, *args, **kwargs): self._linuxscsi = linuxscsi.LinuxSCSI(root_helper, execute=execute) @@ -43,7 +43,7 @@ class BaseLinuxConnector(initiator_connector.InitiatorConnector): *args, **kwargs) @staticmethod - def get_connector_properties(root_helper, *args, **kwargs): + def get_connector_properties(root_helper: str, *args, **kwargs) -> dict: """The generic connector properties.""" multipath = kwargs['multipath'] enforce_multipath = kwargs['enforce_multipath'] @@ -56,7 +56,7 @@ class BaseLinuxConnector(initiator_connector.InitiatorConnector): return props - def check_valid_device(self, path, run_as_root=True): + def check_valid_device(self, path: str, run_as_root: bool = True) -> bool: cmd = ('dd', 'if=%(path)s' % {"path": path}, 'of=/dev/null', 'count=1') out, info = None, None @@ -85,8 +85,10 @@ class BaseLinuxConnector(initiator_connector.InitiatorConnector): return volumes - def _discover_mpath_device(self, device_wwn, connection_properties, - device_name): + def _discover_mpath_device(self, + device_wwn: str, + connection_properties: dict, + device_name: str) -> tuple: """This method discovers a multipath device. Discover a multipath device based on a defined connection_property diff --git a/os_brick/initiator/connectors/iscsi.py b/os_brick/initiator/connectors/iscsi.py index 4bdfb8a01..2723c4271 100644 --- a/os_brick/initiator/connectors/iscsi.py +++ b/os_brick/initiator/connectors/iscsi.py @@ -13,12 +13,13 @@ # under the License. -import collections +from collections import defaultdict import copy import glob import os import re import time +from typing import List, Tuple # noqa: H301 from oslo_concurrency import lockutils from oslo_concurrency import processutils as putils @@ -47,20 +48,21 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): 'cxgb4i', 'qla4xxx', 'ocs', 'iser', 'tcp'] VALID_SESSIONS_PREFIX = ('tcp:', 'iser:') - def __init__(self, root_helper, driver=None, - execute=None, use_multipath=False, - device_scan_attempts=initiator.DEVICE_SCAN_ATTEMPTS_DEFAULT, - transport='default', *args, **kwargs): + def __init__( + self, root_helper: str, driver=None, + execute=None, use_multipath: bool = False, + device_scan_attempts: int = initiator.DEVICE_SCAN_ATTEMPTS_DEFAULT, + transport='default', *args, **kwargs): super(ISCSIConnector, self).__init__( root_helper, driver=driver, execute=execute, device_scan_attempts=device_scan_attempts, - transport=transport, *args, **kwargs) - self.use_multipath = use_multipath - self.transport = self._validate_iface_transport(transport) + transport=transport, *args, **kwargs) # type: ignore + self.use_multipath: bool = use_multipath + self.transport: str = self._validate_iface_transport(transport) @staticmethod - def get_connector_properties(root_helper, *args, **kwargs): + def get_connector_properties(root_helper: str, *args, **kwargs) -> dict: """The iSCSI connector properties.""" props = {} iscsi = ISCSIConnector(root_helper=root_helper, @@ -71,11 +73,11 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): return props - def get_search_path(self): + def get_search_path(self) -> str: """Where do we look for iSCSI based volumes.""" return '/dev/disk/by-path' - def get_volume_paths(self, connection_properties): + def get_volume_paths(self, connection_properties: dict) -> list: """Get the list of existing paths for a volume. This method's job is to simply report what might/should @@ -87,7 +89,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): of the target volume attributes. :type connection_properties: dict """ - volume_paths = [] + volume_paths: list = [] # if there are no sessions, then target_portal won't exist if (('target_portal' not in connection_properties) and @@ -106,7 +108,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): return volume_paths - def _get_iscsi_sessions_full(self): + def _get_iscsi_sessions_full(self) -> List[tuple]: """Get iSCSI session information as a list of tuples. Uses iscsiadm -m session and from a command output like @@ -124,7 +126,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): # Parse and clean the output from iscsiadm, which is in the form of: # transport_name: [session_id] ip_address:port,tpgt iqn node_type - lines = [] + lines: List[tuple] = [] for line in out.splitlines(): if line: info = line.split() @@ -133,7 +135,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): lines.append((info[0], sid, portal, tpgt, info[3])) return lines - def _get_iscsi_nodes(self): + def _get_iscsi_nodes(self) -> List[tuple]: """Get iSCSI node information (portal, iqn) as a list of tuples. Uses iscsiadm -m node and from a command output like @@ -151,8 +153,8 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): return [] # Parse and clean the output from iscsiadm which is in the form of: - # ip_addresss:port,tpgt iqn - lines = [] + # ip_address:port,tpgt iqn + lines: List[tuple] = [] for line in out.splitlines(): if line: info = line.split() @@ -162,13 +164,15 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): pass return lines - def _get_iscsi_sessions(self): + def _get_iscsi_sessions(self) -> list: """Return portals for all existing sessions.""" # entry: [tcp, [1], 192.168.121.250:3260,1 ...] return [entry[2] for entry in self._get_iscsi_sessions_full()] - def _get_ips_iqns_luns(self, connection_properties, discover=True, - is_disconnect_call=False): + def _get_ips_iqns_luns(self, + connection_properties: dict, + discover: bool = True, + is_disconnect_call: bool = False): """Build a list of ips, iqns, and luns. Used when doing singlepath and multipath, and we have 4 cases: @@ -236,7 +240,8 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): return ips_iqns_luns - def _get_potential_volume_paths(self, connection_properties): + def _get_potential_volume_paths(self, + connection_properties: dict) -> List[str]: """Build a list of potential volume paths that exist. Given a list of target_portals in the connection_properties, @@ -279,7 +284,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): super(ISCSIConnector, self).set_execute(execute) self._linuxscsi.set_execute(execute) - def _validate_iface_transport(self, transport_iface): + def _validate_iface_transport(self, transport_iface: str) -> str: """Check that given iscsi_iface uses only supported transports Accepted transport names for provided iface param are @@ -318,10 +323,11 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): transport_iface) return 'default' - def _get_transport(self): + def _get_transport(self) -> str: return self.transport - def _get_discoverydb_portals(self, connection_properties): + def _get_discoverydb_portals(self, + connection_properties: dict) -> List[tuple]: """Retrieve iscsi portals information from the discoverydb. Example of discoverydb command output: @@ -394,7 +400,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): luns = self._get_luns(connection_properties, iqns) return list(zip(ips, iqns, luns)) - def _discover_iscsi_portals(self, connection_properties): + def _discover_iscsi_portals(self, connection_properties: dict) -> list: out = None iscsi_transport = ('iser' if self._get_transport() == 'iser' else 'default') @@ -469,7 +475,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): @utils.trace @synchronized('extend_volume') - def extend_volume(self, connection_properties): + def extend_volume(self, connection_properties: dict): """Update the local kernel's size information. Try and update the local kernel's size information @@ -492,7 +498,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): @utils.trace @synchronized('connect_volume') - def connect_volume(self, connection_properties): + def connect_volume(self, connection_properties: dict): """Attach the volume to instance_name. NOTE: Will retry up to three times to handle the case where c-vol @@ -582,7 +588,8 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): break time.sleep(1) else: - LOG.debug('Could not find the WWN for %s.', found_devs[0]) + LOG.debug('Could not find the WWN for %s.', + found_devs[0]) # type: ignore return self._get_connect_result(connection_properties, wwn, found_devs) @@ -709,9 +716,10 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): was the first time this volume was seen here. """ wwn = mpath = None - wwn_added = last_try_on = False - found = [] - just_added_devices = [] + wwn_added = False + last_try_on = 0.0 + found: list = [] + just_added_devices: list = [] # Dict used to communicate with threads as detailed in _connect_vol data = {'stop_connecting': False, 'num_logins': 0, 'failed_logins': 0, 'stopped_threads': 0, 'found_devices': found, @@ -834,7 +842,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): if s[0] in self.VALID_SESSIONS_PREFIX} # device_map will keep a tuple with devices from the connection and # others that don't belong to this connection" (belong, others) - device_map = collections.defaultdict(lambda: (set(), set())) + device_map: defaultdict = defaultdict(lambda: (set(), set())) for ip, iqn, lun in ips_iqns_luns: session = sessions_map.get((ip, iqn)) @@ -933,13 +941,16 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): was_multipath = (path_used.startswith('/dev/dm-') or 'mpath' in path_used) multipath_name = self._linuxscsi.remove_connection( - remove_devices, force, exc, path_used, was_multipath) + remove_devices, force, + exc, path_used, was_multipath) # type: ignore # Disconnect sessions and remove nodes that are left without devices disconnect = [conn for conn, (__, keep) in devices_map.items() if not keep] + + # The "type:" comment works around mypy issue #6647 self._disconnect_connection(connection_properties, disconnect, force, - exc) + exc) # type:ignore # If flushing the multipath failed before, try now after we have # removed the devices and we may have even logged off (only reaches @@ -949,11 +960,11 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): 'devices.', multipath_name) self._linuxscsi.flush_multipath_device(multipath_name) - if exc: + if exc: # type: ignore LOG.warning('There were errors removing %s, leftovers may remain ' 'in the system', remove_devices) if not ignore_errors: - raise exc + raise exc # type: ignore def _munge_portal(self, target): """Remove brackets from portal. @@ -1133,7 +1144,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): {'out': out, 'err': err}) return (out, err) - def _run_iscsiadm_bare(self, iscsi_command, **kwargs): + def _run_iscsiadm_bare(self, iscsi_command, **kwargs) -> Tuple[str, str]: check_exit_code = kwargs.pop('check_exit_code', 0) (out, err) = self._execute('iscsiadm', *iscsi_command, @@ -1164,8 +1175,8 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): ['-m', 'node', '--op', 'show', '-p', connection_properties['target_portal']], check_exit_code=(0, 21)) or "" - node_values = out.strip() - node_values = node_values.split("\n") + node_values_str = out.strip() + node_values = node_values_str.split("\n") iqn = None startup = None startup_values = {} diff --git a/os_brick/initiator/linuxscsi.py b/os_brick/initiator/linuxscsi.py index a9c7fdd31..ee19febbf 100644 --- a/os_brick/initiator/linuxscsi.py +++ b/os_brick/initiator/linuxscsi.py @@ -20,6 +20,7 @@ import glob import os import re import time +from typing import Dict, List, Optional # noqa: H301 from oslo_concurrency import processutils as putils from oslo_log import log as logging @@ -42,7 +43,7 @@ class LinuxSCSI(executor.Executor): # As found in drivers/scsi/scsi_lib.c WWN_TYPES = {'t10.': '1', 'eui.': '2', 'naa.': '3'} - def echo_scsi_command(self, path, content): + def echo_scsi_command(self, path, content) -> None: """Used to echo strings to scsi subsystem.""" args = ["-a", path] @@ -51,7 +52,7 @@ class LinuxSCSI(executor.Executor): root_helper=self._root_helper) self._execute('tee', *args, **kwargs) - def get_name_from_path(self, path): + def get_name_from_path(self, path) -> Optional[str]: """Translates /dev/disk/by-path/ entry to /dev/sdX.""" name = os.path.realpath(path) @@ -60,8 +61,11 @@ class LinuxSCSI(executor.Executor): else: return None - def remove_scsi_device(self, device, force=False, exc=None, - flush=True): + def remove_scsi_device(self, + device: str, + force: bool = False, + exc=None, + flush: bool = True) -> None: """Removes a scsi device based upon /dev/sdX name.""" path = "/sys/block/%s/device/delete" % device.replace("/dev/", "") if os.path.exists(path): @@ -76,7 +80,7 @@ class LinuxSCSI(executor.Executor): with exc.context(force, 'Removing %s failed', device): self.echo_scsi_command(path, "1") - def wait_for_volumes_removal(self, volumes_names): + def wait_for_volumes_removal(self, volumes_names: List[str]) -> None: """Wait for device paths to be removed from the system.""" str_names = ', '.join(volumes_names) LOG.debug('Checking to see if SCSI volumes %s have been removed.', @@ -99,7 +103,7 @@ class LinuxSCSI(executor.Executor): LOG.debug('%s still exist.', ', '.join(exist)) raise exception.VolumePathNotRemoved(volume_path=exist) - def get_device_info(self, device): + def get_device_info(self, device: str) -> Dict[str, Optional[str]]: dev_info = {'device': device, 'host': None, 'channel': None, 'id': None, 'lun': None} # The input argument 'device' can be of 2 types: @@ -126,7 +130,7 @@ class LinuxSCSI(executor.Executor): LOG.debug('dev_info=%s', str(dev_info)) return dev_info - def get_sysfs_wwn(self, device_names, mpath=None): + def get_sysfs_wwn(self, device_names, mpath=None) -> str: """Return the wwid from sysfs in any of devices in udev format.""" # If we have a multipath DM we know that it has found the WWN if mpath: @@ -201,7 +205,9 @@ class LinuxSCSI(executor.Executor): return out.strip() @staticmethod - def is_multipath_running(enforce_multipath, root_helper, execute=None): + def is_multipath_running(enforce_multipath, + root_helper, + execute=None) -> bool: try: if execute is None: execute = priv_rootwrap.execute @@ -495,8 +501,8 @@ class LinuxSCSI(executor.Executor): cmd='multipath -l %s' % device) if out: - lines = out.strip() - lines = lines.split("\n") + lines_str = out.strip() + lines = lines_str.split("\n") lines = [line for line in lines if not re.match(MULTIPATH_ERROR_REGEX, line) and len(line)] diff --git a/os_brick/remotefs/remotefs.py b/os_brick/remotefs/remotefs.py index c31ff175a..7ae71690f 100644 --- a/os_brick/remotefs/remotefs.py +++ b/os_brick/remotefs/remotefs.py @@ -35,7 +35,7 @@ class RemoteFsClient(executor.Executor): def __init__(self, mount_type, root_helper, execute=None, *args, **kwargs): super(RemoteFsClient, self).__init__(root_helper, execute=execute, - *args, **kwargs) + *args, **kwargs) # type: ignore mount_type_to_option_prefix = { 'nfs': 'nfs', @@ -52,7 +52,9 @@ class RemoteFsClient(executor.Executor): self._mount_type = mount_type option_prefix = mount_type_to_option_prefix[mount_type] - self._mount_base = kwargs.get(option_prefix + '_mount_point_base') + self._mount_base: str + self._mount_base = kwargs.get(option_prefix + + '_mount_point_base') # type: ignore if not self._mount_base: raise exception.InvalidParameterValue( err=_('%s_mount_point_base required') % option_prefix) @@ -72,7 +74,7 @@ class RemoteFsClient(executor.Executor): return md5(base_str, usedforsecurity=False).hexdigest() - def get_mount_point(self, device_name): + def get_mount_point(self, device_name: str): """Get Mount Point. :param device_name: example 172.18.194.100:/var/nfs @@ -197,9 +199,9 @@ class RemoteFsClient(executor.Executor): class ScalityRemoteFsClient(RemoteFsClient): def __init__(self, mount_type, root_helper, execute=None, *args, **kwargs): - super(ScalityRemoteFsClient, self).__init__(mount_type, root_helper, - execute=execute, - *args, **kwargs) + super(ScalityRemoteFsClient, self).__init__( + mount_type, root_helper, execute=execute, + *args, **kwargs) # type: ignore self._mount_type = mount_type self._mount_base = kwargs.get( 'scality_mount_point_base', "").rstrip('/') diff --git a/setup.cfg b/setup.cfg index dbc8b975b..cc6644804 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,3 +31,15 @@ data_files = tag_build = tag_date = 0 tag_svn_revision = 0 + +[mypy] +show_column_numbers = true +show_error_context = true +ignore_missing_imports = true +follow_imports = skip +incremental = true +check_untyped_defs = true +warn_unused_ignores = true +show_error_codes = true +pretty = true +html_report = mypy-report diff --git a/test-requirements.txt b/test-requirements.txt index 96fb5d6f0..1d03eceec 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -16,3 +16,4 @@ pycodestyle==2.6.0 # MIT doc8>=0.8.1 # Apache-2.0 fixtures>=3.0.0 # Apache-2.0/BSD bandit>=1.6.0,<1.7.0 # Apache-2.0 +mypy>=0.782 # MIT diff --git a/tools/mypywrap.sh b/tools/mypywrap.sh new file mode 100644 index 000000000..6ff309ff1 --- /dev/null +++ b/tools/mypywrap.sh @@ -0,0 +1,24 @@ +#!/bin/sh +# +# A wrapper around mypy that allows us to specify what files to run 'mypy' type +# checks on. Intended to be invoked via tox: +# +# tox -e mypy +# +# Eventually this should go away once we have either converted everything or +# converted enough and ignored [1] the rest. +# +# [1] http://mypy.readthedocs.io/en/latest/config_file.html#per-module-flags + +ROOT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" + +export MYPYPATH=$ROOT_DIR/../os_brick/tests/stubs/ + +if [ $# -eq 0 ]; then + # if no arguments provided, use the standard converted lists + lines=$(grep -v '#' $ROOT_DIR/../mypy-files.txt) + python -m mypy ${lines[@]} +else + # else test what the user asked us to + python -m mypy $@ +fi diff --git a/tox.ini b/tox.ini index c9bd2fce3..3f6cf90db 100644 --- a/tox.ini +++ b/tox.ini @@ -143,3 +143,9 @@ deps = -c{toxinidir}/lower-constraints.txt -r{toxinidir}/test-requirements.txt -r{toxinidir}/requirements.txt + +[testenv:mypy] +description = + Run type checks. +commands = + bash tools/mypywrap.sh {posargs}