From: Rishabh Dave Date: Thu, 2 Nov 2023 15:15:27 +0000 (+0530) Subject: mgr/vol: fix flake8 warnings X-Git-Tag: v18.2.4~96^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=719e83baed793ef515641c70d8bea7b236d98e39;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 (cherry picked from commit 95dac209a2e0788dce490f64645ff5d5a17fdfb9) --- diff --git a/src/pybind/mgr/volumes/fs/async_cloner.py b/src/pybind/mgr/volumes/fs/async_cloner.py index deb208477bcf..5761a9cb945e 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 2b419e196a54..097620d73780 100644 --- a/src/pybind/mgr/volumes/fs/operations/versions/__init__.py +++ b/src/pybind/mgr/volumes/fs/operations/versions/__init__.py @@ -98,7 +98,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 d2e1c54f1108..610a61e6a4c1 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 6b377ff42c40..4667653f2d1e 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 f93ecee19a06..0bf42827161e 100644 --- a/src/pybind/mgr/volumes/fs/operations/volume.py +++ b/src/pybind/mgr/volumes/fs/operations/volume.py @@ -41,7 +41,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'] @@ -62,7 +62,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 9f6a2e2a6462..2e96f830636b 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 @@ -18,7 +17,6 @@ from .operations.volume import create_volume, delete_volume, rename_volume, \ get_pending_subvol_deletions_count, get_all_pending_clones_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, \ @@ -341,7 +339,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): @@ -699,7 +697,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) @@ -714,7 +712,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) @@ -970,7 +968,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)