From: Rishabh Dave Date: Thu, 2 Nov 2023 15:15:27 +0000 (+0530) Subject: mgr/vol: fix flake8 warnings X-Git-Tag: v19.0.0~23^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=95dac209a2e0788dce490f64645ff5d5a17fdfb9;p=ceph.git mgr/vol: fix flake8 warnings Fix warnings printed by the command "flake8 --select=F,E9 --exclude=venv,.tox src/pybind/mgr/volumes/". While working with mgr/vol, syntax errors are not printed anywhere. Any attempt to run vstart.sh after such faulty patch causes vstart.sh to go in a infinite loop. And running "ceph fs volume" command prints "no such command exists". This doesn't tell the actual issue and causes confusion. When flake8 is run, the issue is not immediately apparent due to so many warnings. Therefore, fix these warnings so that it becomes easier to spot such critical issues here onwards. Note: mypy uses comments like "#type: Dict" for type checking and therefore counts 'from typechecking import Dict' as not unsued. But flake8 doesn't recognize type hints embedded in comment. Therfore, switch to actually using type hints instead of adding type hints to comments. Signed-off-by: Rishabh Dave --- diff --git a/src/pybind/mgr/volumes/fs/async_cloner.py b/src/pybind/mgr/volumes/fs/async_cloner.py index 95f7d64e1b36..bfa34ed03c02 100644 --- a/src/pybind/mgr/volumes/fs/async_cloner.py +++ b/src/pybind/mgr/volumes/fs/async_cloner.py @@ -191,7 +191,7 @@ def bulk_copy(fs_handle, source_path, dst_path, should_cancel): def set_quota_on_clone(fs_handle, clone_volumes_pair): src_path = clone_volumes_pair[1].snapshot_data_path(clone_volumes_pair[2]) dst_path = clone_volumes_pair[0].path - quota = None # type: Optional[int] + quota: Optional[int] = None try: quota = int(fs_handle.getxattr(src_path, 'ceph.quota.max_bytes').decode('utf-8')) except cephfs.NoData: @@ -205,7 +205,7 @@ def set_quota_on_clone(fs_handle, clone_volumes_pair): except cephfs.Error as e: raise VolumeException(-e.args[0], e.args[1]) - quota_files = None # type: Optional[int] + quota_files: Optional[int] = None try: quota_files = int(fs_handle.getxattr(src_path, 'ceph.quota.max_files').decode('utf-8')) except cephfs.NoData: diff --git a/src/pybind/mgr/volumes/fs/operations/access.py b/src/pybind/mgr/volumes/fs/operations/access.py index 9b7b2431605c..7e916e95539e 100644 --- a/src/pybind/mgr/volumes/fs/operations/access.py +++ b/src/pybind/mgr/volumes/fs/operations/access.py @@ -4,7 +4,7 @@ from typing import List def prepare_updated_caps_list(existing_caps, mds_cap_str, osd_cap_str, authorize=True): - caps_list = [] # type: List[str] + caps_list: List[str] = [] for k, v in existing_caps['caps'].items(): if k == 'mds' or k == 'osd': continue diff --git a/src/pybind/mgr/volumes/fs/operations/lock.py b/src/pybind/mgr/volumes/fs/operations/lock.py index 7ef6923e115c..9588ddec164e 100644 --- a/src/pybind/mgr/volumes/fs/operations/lock.py +++ b/src/pybind/mgr/volumes/fs/operations/lock.py @@ -22,10 +22,10 @@ class GlobalLock(object): See: https://people.eecs.berkeley.edu/~kubitron/courses/cs262a-F14/projects/reports/project6_report.pdf """ - _shared_state = { + _shared_state: Dict = { 'lock' : Lock(), 'init' : False - } # type: Dict + } def __init__(self): with self._shared_state['lock']: diff --git a/src/pybind/mgr/volumes/fs/operations/pin_util.py b/src/pybind/mgr/volumes/fs/operations/pin_util.py index 9ea79e546e26..a12ab5b4d4b2 100644 --- a/src/pybind/mgr/volumes/fs/operations/pin_util.py +++ b/src/pybind/mgr/volumes/fs/operations/pin_util.py @@ -1,4 +1,3 @@ -import os import errno import cephfs @@ -25,7 +24,7 @@ def pin(fs, path, pin_type, pin_setting): try: pin_setting = _pin_value[pin_type](pin_setting) - except ValueError as e: + except ValueError: raise VolumeException(-errno.EINVAL, f"pin value wrong type: {pin_setting}") try: diff --git a/src/pybind/mgr/volumes/fs/operations/trash.py b/src/pybind/mgr/volumes/fs/operations/trash.py index 66f1d71cf89a..d76d43a43d13 100644 --- a/src/pybind/mgr/volumes/fs/operations/trash.py +++ b/src/pybind/mgr/volumes/fs/operations/trash.py @@ -6,7 +6,6 @@ from contextlib import contextmanager import cephfs from .template import GroupTemplate -from ..fs_util import listdir from ..exception import VolumeException log = logging.getLogger(__name__) diff --git a/src/pybind/mgr/volumes/fs/operations/versions/__init__.py b/src/pybind/mgr/volumes/fs/operations/versions/__init__.py index 544afa165f97..6c96c4570199 100644 --- a/src/pybind/mgr/volumes/fs/operations/versions/__init__.py +++ b/src/pybind/mgr/volumes/fs/operations/versions/__init__.py @@ -83,7 +83,7 @@ class SubvolumeLoader(object): subvolume_type = SubvolumeTypes.TYPE_NORMAL try: initial_state = SubvolumeOpSm.get_init_state(subvolume_type) - except OpSmException as oe: + except OpSmException: raise VolumeException(-errno.EINVAL, "subvolume creation failed: internal error") qpath = subvolume.base_path.decode('utf-8') # legacy is only upgradable to v1 diff --git a/src/pybind/mgr/volumes/fs/operations/versions/metadata_manager.py b/src/pybind/mgr/volumes/fs/operations/versions/metadata_manager.py index 718735d91b13..b33a2b48b775 100644 --- a/src/pybind/mgr/volumes/fs/operations/versions/metadata_manager.py +++ b/src/pybind/mgr/volumes/fs/operations/versions/metadata_manager.py @@ -1,7 +1,6 @@ import os import errno import logging -import sys import threading import configparser import re diff --git a/src/pybind/mgr/volumes/fs/operations/versions/op_sm.py b/src/pybind/mgr/volumes/fs/operations/versions/op_sm.py index 1142600cbb20..93eafb2bde4c 100644 --- a/src/pybind/mgr/volumes/fs/operations/versions/op_sm.py +++ b/src/pybind/mgr/volumes/fs/operations/versions/op_sm.py @@ -19,7 +19,7 @@ class TransitionKey(object): return not(self == other) class SubvolumeOpSm(object): - transition_table = {} # type: Dict + transition_table: Dict = {} @staticmethod def is_complete_state(state): diff --git a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_base.py b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_base.py index 3bae0707a6a4..8fbe177e5f4b 100644 --- a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_base.py +++ b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_base.py @@ -144,7 +144,7 @@ class SubvolumeBase(object): try: self.fs.stat(self.legacy_config_path) self.legacy_mode = True - except cephfs.Error as e: + except cephfs.Error: pass log.debug("loading config " @@ -160,7 +160,7 @@ class SubvolumeBase(object): def get_attrs(self, pathname): # get subvolume attributes - attrs = {} # type: Dict[str, Union[int, str, None]] + attrs: Dict[str, Union[int, str, None]] = {} stx = self.fs.statx(pathname, cephfs.CEPH_STATX_UID | cephfs.CEPH_STATX_GID | cephfs.CEPH_STATX_MODE, diff --git a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v1.py b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v1.py index b5a10dd6c7f6..c07f835421e5 100644 --- a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v1.py +++ b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v1.py @@ -55,7 +55,7 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate): try: # no need to stat the path -- open() does that return self.metadata_mgr.get_global_option(MetadataManager.GLOBAL_META_KEY_PATH).encode('utf-8') - except MetadataMgrException as me: + except MetadataMgrException: raise VolumeException(-errno.EINVAL, "error fetching subvolume metadata") @property @@ -68,7 +68,7 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate): try: # MDS treats this as a noop for already marked subvolume self.fs.setxattr(self.path, 'ceph.dir.subvolume', b'1', 0) - except cephfs.InvalidValue as e: + except cephfs.InvalidValue: raise VolumeException(-errno.EINVAL, "invalid value specified for ceph.dir.subvolume") except cephfs.Error as e: raise VolumeException(-e.args[0], e.args[1]) @@ -89,7 +89,7 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate): subvolume_type = SubvolumeTypes.TYPE_NORMAL try: initial_state = SubvolumeOpSm.get_init_state(subvolume_type) - except OpSmException as oe: + except OpSmException: raise VolumeException(-errno.EINVAL, "subvolume creation failed: internal error") subvol_path = os.path.join(self.base_path, str(uuid.uuid4()).encode('utf-8')) @@ -156,7 +156,7 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate): subvolume_type = SubvolumeTypes.TYPE_CLONE try: initial_state = SubvolumeOpSm.get_init_state(subvolume_type) - except OpSmException as oe: + except OpSmException: raise VolumeException(-errno.EINVAL, "clone failed: internal error") subvol_path = os.path.join(self.base_path, str(uuid.uuid4()).encode('utf-8')) @@ -596,7 +596,7 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate): """ with self.auth_mdata_mgr.subvol_metadata_lock(self.group.groupname, self.subvolname): meta = self.auth_mdata_mgr.subvol_metadata_get(self.group.groupname, self.subvolname) - auths = [] # type: List[Dict[str,str]] + auths: List[Dict[str,str]] = [] if not meta or not meta['auths']: return auths @@ -669,7 +669,7 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate): pass else: raise - except MetadataMgrException as me: + except MetadataMgrException: raise VolumeException(-errno.EINVAL, "error fetching subvolume metadata") return clone_source @@ -744,7 +744,7 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate): raise def get_pending_clones(self, snapname): - pending_clones_info = {"has_pending_clones": "no"} # type: Dict[str, Any] + pending_clones_info: Dict[str, Any] = {"has_pending_clones": "no"} pending_track_id_list = [] pending_clone_list = [] index_path = "" @@ -777,7 +777,6 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate): # If clone is completed between 'list_all_keys_with_specified_values_from_section' # and readlink(track_id_path) call then readlink will fail with error ENOENT (2) # Hence we double check whether track_id is exist in .meta file or not. - value = self.metadata_mgr.get_option('clone snaps', track_id) # Edge case scenario. # If track_id for clone exist but path /volumes/_index/clone/{track_id} not found # then clone is orphan. @@ -790,7 +789,7 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate): path = Path(link_path.decode('utf-8')) clone_name = os.path.basename(link_path).decode('utf-8') group_name = os.path.basename(path.parent.absolute()) - details = {"name": clone_name} # type: Dict[str, str] + details = {"name": clone_name} if group_name != Group.NO_GROUP_NAME: details["target_group"] = group_name pending_clone_list.append(details) @@ -839,7 +838,7 @@ class SubvolumeV1(SubvolumeBase, SubvolumeTemplate): snap_info[key] = self.fs.getxattr(snappath, val) pending_clones_info = self.get_pending_clones(snapname) info_dict = {'created_at': str(datetime.fromtimestamp(float(snap_info['created_at']))), - 'data_pool': snap_info['data_pool'].decode('utf-8')} # type: Dict[str, Any] + 'data_pool': snap_info['data_pool'].decode('utf-8')} info_dict.update(pending_clones_info); return info_dict except cephfs.Error as e: diff --git a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py index 03085d049713..17955d3020f8 100644 --- a/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py +++ b/src/pybind/mgr/volumes/fs/operations/versions/subvolume_v2.py @@ -10,7 +10,6 @@ from .metadata_manager import MetadataManager from .subvolume_attrs import SubvolumeTypes, SubvolumeStates, SubvolumeFeatures from .op_sm import SubvolumeOpSm from .subvolume_v1 import SubvolumeV1 -from ..template import SubvolumeTemplate from ...exception import OpSmException, VolumeException, MetadataMgrException from ...fs_util import listdir, create_base_dir from ..template import SubvolumeOpType @@ -99,7 +98,7 @@ class SubvolumeV2(SubvolumeV1): try: # MDS treats this as a noop for already marked subvolume self.fs.setxattr(self.base_path, 'ceph.dir.subvolume', b'1', 0) - except cephfs.InvalidValue as e: + except cephfs.InvalidValue: raise VolumeException(-errno.EINVAL, "invalid value specified for ceph.dir.subvolume") except cephfs.Error as e: raise VolumeException(-e.args[0], e.args[1]) @@ -159,7 +158,7 @@ class SubvolumeV2(SubvolumeV1): subvolume_type = SubvolumeTypes.TYPE_NORMAL try: initial_state = SubvolumeOpSm.get_init_state(subvolume_type) - except OpSmException as oe: + except OpSmException: raise VolumeException(-errno.EINVAL, "subvolume creation failed: internal error") retained = self.retained @@ -207,7 +206,7 @@ class SubvolumeV2(SubvolumeV1): subvolume_type = SubvolumeTypes.TYPE_CLONE try: initial_state = SubvolumeOpSm.get_init_state(subvolume_type) - except OpSmException as oe: + except OpSmException: raise VolumeException(-errno.EINVAL, "clone failed: internal error") retained = self.retained diff --git a/src/pybind/mgr/volumes/fs/operations/volume.py b/src/pybind/mgr/volumes/fs/operations/volume.py index 395a3fb4ea07..67fbb891cefb 100644 --- a/src/pybind/mgr/volumes/fs/operations/volume.py +++ b/src/pybind/mgr/volumes/fs/operations/volume.py @@ -40,7 +40,7 @@ def get_pool_names(mgr, volname): """ fs_map = mgr.get("fs_map") metadata_pool_id = None - data_pool_ids = [] # type: List[int] + data_pool_ids: List[int] = [] for f in fs_map['filesystems']: if volname == f['mdsmap']['fs_name']: metadata_pool_id = f['mdsmap']['metadata_pool'] @@ -61,7 +61,7 @@ def get_pool_ids(mgr, volname): """ fs_map = mgr.get("fs_map") metadata_pool_id = None - data_pool_ids = [] # type: List[int] + data_pool_ids: List[int] = [] for f in fs_map['filesystems']: if volname == f['mdsmap']['fs_name']: metadata_pool_id = f['mdsmap']['metadata_pool'] diff --git a/src/pybind/mgr/volumes/fs/volume.py b/src/pybind/mgr/volumes/fs/volume.py index 5c6642444b11..e19d1eafb2a6 100644 --- a/src/pybind/mgr/volumes/fs/volume.py +++ b/src/pybind/mgr/volumes/fs/volume.py @@ -1,7 +1,6 @@ import json import errno import logging -import os import mgr_util from typing import TYPE_CHECKING @@ -17,7 +16,6 @@ from .operations.volume import create_volume, delete_volume, rename_volume, \ list_volumes, open_volume, get_pool_names, get_pool_ids, get_pending_subvol_deletions_count from .operations.subvolume import open_subvol, create_subvol, remove_subvol, \ create_clone -from .operations.trash import Trash from .vol_spec import VolSpec from .exception import VolumeException, ClusterError, ClusterTimeout, EvictionError @@ -338,7 +336,7 @@ class VolumeClient(CephfsClient["Module"]): with open_volume(self, volname) as fs_handle: with open_group(fs_handle, self.volspec, groupname) as group: with open_subvol(self.mgr, fs_handle, self.volspec, group, subvolname, SubvolumeOpType.EVICT) as subvolume: - key = subvolume.evict(volname, authid) + subvolume.evict(volname, authid) ret = 0, "", "" except (VolumeException, ClusterTimeout, ClusterError, EvictionError) as e: if isinstance(e, VolumeException): @@ -695,7 +693,7 @@ class VolumeClient(CephfsClient["Module"]): try: with open_volume(self, volname) as fs_handle: with open_group(fs_handle, self.volspec, groupname) as group: - with open_subvol(self.mgr, fs_handle, self.volspec, group, subvolname, SubvolumeOpType.SNAP_PROTECT) as subvolume: + with open_subvol(self.mgr, fs_handle, self.volspec, group, subvolname, SubvolumeOpType.SNAP_PROTECT): log.warning("snapshot protect call is deprecated and will be removed in a future release") except VolumeException as ve: ret = self.volume_exception_to_retval(ve) @@ -710,7 +708,7 @@ class VolumeClient(CephfsClient["Module"]): try: with open_volume(self, volname) as fs_handle: with open_group(fs_handle, self.volspec, groupname) as group: - with open_subvol(self.mgr, fs_handle, self.volspec, group, subvolname, SubvolumeOpType.SNAP_UNPROTECT) as subvolume: + with open_subvol(self.mgr, fs_handle, self.volspec, group, subvolname, SubvolumeOpType.SNAP_UNPROTECT): log.warning("snapshot unprotect call is deprecated and will be removed in a future release") except VolumeException as ve: ret = self.volume_exception_to_retval(ve) @@ -962,7 +960,7 @@ class VolumeClient(CephfsClient["Module"]): try: with open_volume(self, volname) as fs_handle: - with open_group(fs_handle, self.volspec, groupname) as group: + with open_group(fs_handle, self.volspec, groupname): # as subvolumes are marked with the vxattr ceph.dir.subvolume deny snapshots # at the subvolume group (see: https://tracker.ceph.com/issues/46074) # group.create_snapshot(snapname)