From 90f9a98c8cdbff957414ee7952b4632a2d387bb8 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Sat, 20 Jan 2024 11:22:51 -0500 Subject: [PATCH] pybind/mgr/rook: use AllMatcher when size is not provided Issue found by mypy 1.6.1. The previous code was: `and ((sizematcher != None) or sizematcher.compare(device)` meaning that if sizematcher is not none "return" true, but if sizematcher is not none execute the compare method. This is of course impossible as None will never have a compare method. I assume that the At Kefu Chai's suggestion we can replace the None condition with an AllMatcher object when no size is provided to create a size matcher. This both corrects the error and makes the code much simpler and more readable. Signed-off-by: John Mulligan --- src/pybind/mgr/rook/rook_cluster.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/pybind/mgr/rook/rook_cluster.py b/src/pybind/mgr/rook/rook_cluster.py index 21a8e77ba1ff1..c7d2a34a7b755 100644 --- a/src/pybind/mgr/rook/rook_cluster.py +++ b/src/pybind/mgr/rook/rook_cluster.py @@ -33,7 +33,11 @@ from ceph.deployment.service_spec import ( HostPattern, ) from ceph.utils import datetime_now -from ceph.deployment.drive_selection.matchers import SizeMatcher +from ceph.deployment.drive_selection.matchers import ( + AllMatcher, + Matcher, + SizeMatcher, +) from nfs.cluster import create_ganesha_pool from nfs.module import Module from nfs.export import NFSRados @@ -416,7 +420,7 @@ class DefaultCreator(): def filter_devices(self, rook_pods: KubernetesResource, drive_group: DriveGroupSpec, matching_hosts: List[str]) -> List[Device]: device_list = [] assert drive_group.data_devices is not None - sizematcher: Optional[SizeMatcher] = None + sizematcher: Matcher = AllMatcher('', None) if drive_group.data_devices.size: sizematcher = SizeMatcher('size', drive_group.data_devices.size) limit = getattr(drive_group.data_devices, 'limit', None) @@ -444,7 +448,7 @@ class DefaultCreator(): all or ( device.sys_api['node'] in matching_hosts - and ((sizematcher != None) or sizematcher.compare(device)) + and sizematcher.compare(device) and ( not drive_group.data_devices.paths or (device.path in paths) @@ -481,7 +485,7 @@ class LSOCreator(DefaultCreator): def filter_devices(self, rook_pods: KubernetesResource, drive_group: DriveGroupSpec, matching_hosts: List[str]) -> List[Device]: device_list = [] assert drive_group.data_devices is not None - sizematcher = None + sizematcher: Matcher = AllMatcher('', None) if drive_group.data_devices.size: sizematcher = SizeMatcher('size', drive_group.data_devices.size) limit = getattr(drive_group.data_devices, 'limit', None) @@ -511,7 +515,7 @@ class LSOCreator(DefaultCreator): all or ( device.sys_api['node'] in matching_hosts - and ((sizematcher != None) or sizematcher.compare(device)) + and sizematcher.compare(device) and ( not drive_group.data_devices.paths or device.path in paths -- 2.39.5