From 582f7c1657766579acc69512bf7a90088e5618a2 Mon Sep 17 00:00:00 2001 From: Sebastian Wagner Date: Mon, 1 Feb 2021 17:08:28 +0100 Subject: [PATCH] python-common: Set disallow_untyped_defs = True Signed-off-by: Sebastian Wagner --- src/mypy.ini | 7 ++ src/pybind/mgr/cephadm/migrations.py | 16 ++-- .../ceph/deployment/drive_group.py | 14 ++-- .../deployment/drive_selection/matchers.py | 9 +- src/python-common/ceph/deployment/hostspec.py | 10 +-- .../ceph/deployment/inventory.py | 2 +- .../ceph/deployment/service_spec.py | 83 ++++++++++--------- src/python-common/tox.ini | 3 +- 8 files changed, 79 insertions(+), 65 deletions(-) diff --git a/src/mypy.ini b/src/mypy.ini index a10f9e4b3f2..3adb514b64a 100755 --- a/src/mypy.ini +++ b/src/mypy.ini @@ -18,6 +18,13 @@ ignore_missing_imports = True # This would require a cephfs.pyi file ignore_missing_imports = True +# python-common +[mypy-ceph.*] +disallow_untyped_defs = True +[mypy-ceph.tests.*] +disallow_untyped_defs = False +ignore_errors = True + [mypy-mgr_util] disallow_untyped_defs = True diff --git a/src/pybind/mgr/cephadm/migrations.py b/src/pybind/mgr/cephadm/migrations.py index ee16be58836..05df6db282a 100644 --- a/src/pybind/mgr/cephadm/migrations.py +++ b/src/pybind/mgr/cephadm/migrations.py @@ -1,10 +1,10 @@ import logging -from typing import TYPE_CHECKING, Iterator +from typing import TYPE_CHECKING, Iterator, Union from ceph.deployment.service_spec import PlacementSpec, ServiceSpec, HostPlacementSpec from cephadm.schedule import HostAssignment -from orchestrator import OrchestratorError +from orchestrator import OrchestratorError, DaemonDescription if TYPE_CHECKING: from .module import CephadmOrchestrator @@ -101,11 +101,15 @@ class Migrations: if len(placements) >= len(existing_daemons): return + def to_hostname(d: DaemonDescription) -> HostPlacementSpec: + if d.hostname in old_hosts: + return old_hosts[d.hostname] + else: + assert d.hostname + return HostPlacementSpec(d.hostname, '', '') + old_hosts = {h.hostname: h for h in spec.placement.hosts} - new_hosts = [ - old_hosts[d.hostname] if d.hostname in old_hosts else HostPlacementSpec( - hostname=d.hostname, network='', name='') - for d in existing_daemons + new_hosts = [to_hostname(d) for d in existing_daemons ] new_placement = PlacementSpec( diff --git a/src/python-common/ceph/deployment/drive_group.py b/src/python-common/ceph/deployment/drive_group.py index 404ea6ed89d..b17749ed156 100644 --- a/src/python-common/ceph/deployment/drive_group.py +++ b/src/python-common/ceph/deployment/drive_group.py @@ -107,7 +107,7 @@ class DeviceSelection(object): return ret - def __repr__(self): + def __repr__(self) -> str: keys = [ key for key in self._supported_filters + ['limit'] if getattr(self, key) is not None ] @@ -117,7 +117,7 @@ class DeviceSelection(object): ', '.join('{}={}'.format(key, repr(getattr(self, key))) for key in keys) ) - def __eq__(self, other): + def __eq__(self, other: Any) -> bool: return repr(self) == repr(other) @@ -127,7 +127,7 @@ class DriveGroupValidationError(ServiceSpecValidationError): if it was raised in a different mgr module. """ - def __init__(self, msg): + def __init__(self, msg: str): super(DriveGroupValidationError, self).__init__('Failed to validate Drive Group: ' + msg) @@ -147,7 +147,7 @@ class DriveGroupSpec(ServiceSpec): def __init__(self, placement=None, # type: Optional[PlacementSpec] - service_id=None, # type: str + service_id=None, # type: Optional[str] data_devices=None, # type: Optional[DeviceSelection] db_devices=None, # type: Optional[DeviceSelection] wal_devices=None, # type: Optional[DeviceSelection] @@ -233,7 +233,7 @@ class DriveGroupSpec(ServiceSpec): :param json_drive_group: A valid json string with a Drive Group specification """ - args = {} + args: Dict[str, Any] = {} # legacy json (pre Octopus) if 'host_pattern' in json_drive_group and 'placement' not in json_drive_group: json_drive_group['placement'] = {'host_pattern': json_drive_group['host_pattern']} @@ -307,7 +307,7 @@ class DriveGroupSpec(ServiceSpec): if self.filter_logic not in ['AND', 'OR']: raise DriveGroupValidationError('filter_logic must be either or ') - def __repr__(self): + def __repr__(self) -> str: keys = [ key for key in self._supported_features if getattr(self, key) is not None ] @@ -320,7 +320,7 @@ class DriveGroupSpec(ServiceSpec): ', '.join('{}={}'.format(key, repr(getattr(self, key))) for key in keys) ) - def __eq__(self, other): + def __eq__(self, other: Any) -> bool: return repr(self) == repr(other) diff --git a/src/python-common/ceph/deployment/drive_selection/matchers.py b/src/python-common/ceph/deployment/drive_selection/matchers.py index 55a7e1090d1..70bd4c38b11 100644 --- a/src/python-common/ceph/deployment/drive_selection/matchers.py +++ b/src/python-common/ceph/deployment/drive_selection/matchers.py @@ -1,9 +1,6 @@ # -*- coding: utf-8 -*- -try: - from typing import Tuple, Optional, Any -except ImportError: - pass +from typing import Tuple, Optional, Any, Union, Iterator from ceph.deployment.inventory import Device @@ -53,7 +50,7 @@ class Matcher(object): # hence, make it a dict. disk = device.to_json() - def findkeys(node, key_val): + def findkeys(node: Union[list, dict], key_val: str) -> Iterator[str]: """ Find keys in non-flat dict recursively """ if isinstance(node, list): for i in node: @@ -291,7 +288,7 @@ class SizeMatcher(Matcher): """ return re.findall(r"\d+", data)[0], cls._parse_suffix(data) - def _parse_filter(self): + def _parse_filter(self) -> None: """ Identifies which type of 'size' filter is applied There are four different filtering modes: diff --git a/src/python-common/ceph/deployment/hostspec.py b/src/python-common/ceph/deployment/hostspec.py index c5ab864182f..714b8536e4e 100644 --- a/src/python-common/ceph/deployment/hostspec.py +++ b/src/python-common/ceph/deployment/hostspec.py @@ -28,7 +28,7 @@ class HostSpec(object): #: human readable status self.status = status or '' # type: str - def to_json(self): + def to_json(self) -> dict: return { 'hostname': self.hostname, 'addr': self.addr, @@ -37,14 +37,14 @@ class HostSpec(object): } @classmethod - def from_json(cls, host_spec): + def from_json(cls, host_spec: dict) -> 'HostSpec': _cls = cls(host_spec['hostname'], host_spec['addr'] if 'addr' in host_spec else None, host_spec['labels'] if 'labels' in host_spec else None, host_spec['status'] if 'status' in host_spec else None) return _cls - def __repr__(self): + def __repr__(self) -> str: args = [self.hostname] # type: List[Any] if self.addr is not None: args.append(self.addr) @@ -55,12 +55,12 @@ class HostSpec(object): return "HostSpec({})".format(', '.join(map(repr, args))) - def __str__(self): + def __str__(self) -> str: if self.hostname != self.addr: return f'{self.hostname} ({self.addr})' return self.hostname - def __eq__(self, other): + def __eq__(self, other: Any) -> bool: # Let's omit `status` for the moment, as it is still the very same host. return self.hostname == other.hostname and \ self.addr == other.addr and \ diff --git a/src/python-common/ceph/deployment/inventory.py b/src/python-common/ceph/deployment/inventory.py index e2409c932e9..8be216391b7 100644 --- a/src/python-common/ceph/deployment/inventory.py +++ b/src/python-common/ceph/deployment/inventory.py @@ -15,7 +15,7 @@ class Devices(object): # type: (List[Device]) -> None self.devices = devices # type: List[Device] - def __eq__(self, other): + def __eq__(self, other: Any) -> bool: return self.to_json() == other.to_json() def to_json(self): diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index f10823a78c3..68b456393e6 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -1,9 +1,10 @@ import errno import fnmatch import re -from collections import namedtuple, OrderedDict +from collections import OrderedDict from functools import wraps -from typing import Optional, Dict, Any, List, Union, Callable, Iterable, Type, TypeVar, cast +from typing import Optional, Dict, Any, List, Union, Callable, Iterable, Type, TypeVar, cast, \ + NamedTuple import yaml @@ -26,7 +27,7 @@ class ServiceSpecValidationError(Exception): self.errno = errno -def assert_valid_host(name): +def assert_valid_host(name: str) -> None: p = re.compile('^[a-zA-Z0-9-]+$') try: assert len(name) <= 250, 'name is too long (max 250 chars)' @@ -35,12 +36,12 @@ def assert_valid_host(name): assert len(part) <= 63, '.-delimited name component must not be more than 63 chars' assert p.match(part), 'name component must include only a-z, 0-9, and -' except AssertionError as e: - raise ServiceSpecValidationError(e) + raise ServiceSpecValidationError(str(e)) def handle_type_error(method: FuncT) -> FuncT: @wraps(method) - def inner(cls, *args, **kwargs): + def inner(cls: Any, *args: Any, **kwargs: Any) -> Any: try: return method(cls, *args, **kwargs) except (TypeError, AttributeError) as e: @@ -49,8 +50,12 @@ def handle_type_error(method: FuncT) -> FuncT: return cast(FuncT, inner) -class HostPlacementSpec(namedtuple('HostPlacementSpec', ['hostname', 'network', 'name'])): - def __str__(self): +class HostPlacementSpec(NamedTuple): + hostname: str + network: str + name: str + + def __str__(self) -> str: res = '' res += self.hostname if self.network: @@ -61,7 +66,7 @@ class HostPlacementSpec(namedtuple('HostPlacementSpec', ['hostname', 'network', @classmethod @handle_type_error - def from_json(cls, data): + def from_json(cls, data: Union[dict, str]) -> 'HostPlacementSpec': if isinstance(data, str): return cls.parse(data) return cls(**data) @@ -139,7 +144,7 @@ class HostPlacementSpec(namedtuple('HostPlacementSpec', ['hostname', 'network', host_spec.validate() return host_spec - def validate(self): + def validate(self) -> None: assert_valid_host(self.hostname) @@ -150,7 +155,7 @@ class PlacementSpec(object): def __init__(self, label=None, # type: Optional[str] - hosts=None, # type: Union[List[str],List[HostPlacementSpec]] + hosts=None, # type: Union[List[str],List[HostPlacementSpec], None] count=None, # type: Optional[int] host_pattern=None # type: Optional[str] ): @@ -168,13 +173,13 @@ class PlacementSpec(object): self.validate() - def is_empty(self): + def is_empty(self) -> bool: return self.label is None and \ not self.hosts and \ not self.host_pattern and \ self.count is None - def __eq__(self, other): + def __eq__(self, other: Any) -> bool: if isinstance(other, PlacementSpec): return self.label == other.label \ and self.hosts == other.hosts \ @@ -182,7 +187,7 @@ class PlacementSpec(object): and self.host_pattern == other.host_pattern return NotImplemented - def set_hosts(self, hosts): + def set_hosts(self, hosts: Union[List[str], List[HostPlacementSpec]]) -> None: # To backpopulate the .hosts attribute when using labels or count # in the orchestrator backend. if all([isinstance(host, HostPlacementSpec) for host in hosts]): @@ -209,12 +214,12 @@ class PlacementSpec(object): # get_host_selection_size return [] - def get_host_selection_size(self, hostspecs: Iterable[HostSpec]): + def get_host_selection_size(self, hostspecs: Iterable[HostSpec]) -> int: if self.count: return self.count return len(self.filter_matching_hostspecs(hostspecs)) - def pretty_str(self): + def pretty_str(self) -> str: """ >>> #doctest: +SKIP ... ps = PlacementSpec(...) # For all placement specs: @@ -231,7 +236,7 @@ class PlacementSpec(object): kv.append(self.host_pattern) return ';'.join(kv) - def __repr__(self): + def __repr__(self) -> str: kv = [] if self.count: kv.append('count=%d' % self.count) @@ -245,7 +250,7 @@ class PlacementSpec(object): @classmethod @handle_type_error - def from_json(cls, data): + def from_json(cls, data: dict) -> 'PlacementSpec': c = data.copy() hosts = c.get('hosts', []) if hosts: @@ -256,8 +261,8 @@ class PlacementSpec(object): _cls.validate() return _cls - def to_json(self): - r = {} + def to_json(self) -> dict: + r: Dict[str, Any] = {} if self.label: r['label'] = self.label if self.hosts: @@ -268,7 +273,7 @@ class PlacementSpec(object): r['host_pattern'] = self.host_pattern return r - def validate(self): + def validate(self) -> None: if self.hosts and self.label: # TODO: a less generic Exception raise ServiceSpecValidationError('Host and label are mutually exclusive') @@ -388,7 +393,7 @@ class ServiceSpec(object): REQUIRES_SERVICE_ID = 'iscsi mds nfs osd rgw container ha-rgw '.split() @classmethod - def _cls(cls, service_type): + def _cls(cls: Type[ServiceSpecT], service_type: str) -> Type[ServiceSpecT]: from ceph.deployment.drive_group import DriveGroupSpec ret = { @@ -404,7 +409,7 @@ class ServiceSpec(object): raise ServiceSpecValidationError('Spec needs a "service_type" key.') return ret - def __new__(cls, *args, **kwargs): + def __new__(cls: Type[ServiceSpecT], *args: Any, **kwargs: Any) -> ServiceSpecT: """ Some Python foo to make sure, we don't have an object like `ServiceSpec('rgw')` of type `ServiceSpec`. Now we have: @@ -416,7 +421,7 @@ class ServiceSpec(object): if cls != ServiceSpec: return object.__new__(cls) service_type = kwargs.get('service_type', args[0] if args else None) - sub_cls = cls._cls(service_type) + sub_cls: Any = cls._cls(service_type) return object.__new__(sub_cls) def __init__(self, @@ -496,8 +501,8 @@ class ServiceSpec(object): return _cls._from_json_impl(c) # type: ignore @classmethod - def _from_json_impl(cls, json_spec): - args = {} # type: Dict[str, Dict[Any, Any]] + def _from_json_impl(cls: Type[ServiceSpecT], json_spec: dict) -> ServiceSpecT: + args = {} # type: Dict[str, Any] for k, v in json_spec.items(): if k == 'placement': v = PlacementSpec.from_json(v) @@ -509,7 +514,7 @@ class ServiceSpec(object): _cls.validate() return _cls - def service_name(self): + def service_name(self) -> str: n = self.service_type if self.service_id: n += '.' + self.service_id @@ -538,7 +543,7 @@ class ServiceSpec(object): ret['spec'] = c return ret - def validate(self): + def validate(self) -> None: if not self.service_type: raise ServiceSpecValidationError('Cannot add Service: type required') @@ -552,19 +557,19 @@ class ServiceSpec(object): if self.placement is not None: self.placement.validate() - def __repr__(self): + def __repr__(self) -> str: return "{}({!r})".format(self.__class__.__name__, self.__dict__) - def __eq__(self, other): + def __eq__(self, other: Any) -> bool: return (self.__class__ == other.__class__ and self.__dict__ == other.__dict__) - def one_line_str(self): + def one_line_str(self) -> str: return '<{} for service_name={}>'.format(self.__class__.__name__, self.service_name()) @staticmethod - def yaml_representer(dumper: 'yaml.SafeDumper', data: 'ServiceSpec'): + def yaml_representer(dumper: 'yaml.SafeDumper', data: 'ServiceSpec') -> Any: return dumper.represent_dict(data.to_json().items()) @@ -592,7 +597,7 @@ class NFSServiceSpec(ServiceSpec): #: RADOS namespace where NFS client recovery data is stored in the pool. self.namespace = namespace - def validate(self): + def validate(self) -> None: super(NFSServiceSpec, self).validate() if not self.pool: @@ -662,7 +667,7 @@ class RGWSpec(ServiceSpec): self.rgw_frontend_ssl_key = rgw_frontend_ssl_key self.ssl = ssl - def get_port(self): + def get_port(self) -> int: if self.rgw_frontend_port: return self.rgw_frontend_port if self.ssl: @@ -670,7 +675,7 @@ class RGWSpec(ServiceSpec): else: return 80 - def rgw_frontends_config_value(self): + def rgw_frontends_config_value(self) -> str: ports = [] if self.ssl: ports.append(f"ssl_port={self.get_port()}") @@ -680,7 +685,7 @@ class RGWSpec(ServiceSpec): ports.append(f"port={self.get_port()}") return f'beast {" ".join(ports)}' - def validate(self): + def validate(self) -> None: super(RGWSpec, self).validate() if not self.rgw_realm: @@ -728,7 +733,7 @@ class IscsiServiceSpec(ServiceSpec): if not self.api_secure and self.ssl_cert and self.ssl_key: self.api_secure = True - def validate(self): + def validate(self) -> None: super(IscsiServiceSpec, self).validate() if not self.pool: @@ -829,7 +834,7 @@ class HA_RGWSpec(ServiceSpec): # when applying spec self.definitive_host_list = [] # type: List[HostPlacementSpec] - def validate(self): + def validate(self) -> None: super(HA_RGWSpec, self).validate() if not self.virtual_ip_interface: @@ -872,11 +877,11 @@ class HA_RGWSpec(ServiceSpec): class CustomContainerSpec(ServiceSpec): def __init__(self, service_type: str = 'container', - service_id: str = None, + service_id: Optional[str] = None, placement: Optional[PlacementSpec] = None, unmanaged: bool = False, preview_only: bool = False, - image: str = None, + image: Optional[str] = None, entrypoint: Optional[str] = None, uid: Optional[int] = None, gid: Optional[int] = None, diff --git a/src/python-common/tox.ini b/src/python-common/tox.ini index da7037b2d81..ce54d5a625f 100644 --- a/src/python-common/tox.ini +++ b/src/python-common/tox.ini @@ -7,7 +7,8 @@ deps= -rrequirements.txt commands= pytest --doctest-modules ceph/deployment/service_spec.py - pytest --mypy --mypy-ignore-missing-imports {posargs} + pytest {posargs} + mypy --config-file=../mypy.ini -p ceph [tool:pytest] norecursedirs = .* _* virtualenv -- 2.39.5