From 2e2ead0c3c81fb030ad75eb596e0ee900acee3a0 Mon Sep 17 00:00:00 2001 From: svalouch Date: Tue, 6 Oct 2020 21:02:57 +0200 Subject: [PATCH] introduce pylint and address some warnings --- setup.cfg | 9 ++++++++ src/simplezfs/exceptions.py | 7 ------- src/simplezfs/pe_helper.py | 6 +++++- src/simplezfs/types.py | 39 ++++++++++++++++------------------- src/simplezfs/validation.py | 2 +- src/simplezfs/zfs.py | 30 +++++++++++++-------------- src/simplezfs/zfs_cli.py | 17 +++++++-------- src/simplezfs/zpool.py | 2 +- src/simplezfs/zpool_cli.py | 6 ++++++ src/simplezfs/zpool_native.py | 6 +++++- 10 files changed, 68 insertions(+), 56 deletions(-) diff --git a/setup.cfg b/setup.cfg index 2cbca99..dfb21de 100644 --- a/setup.cfg +++ b/setup.cfg @@ -23,3 +23,12 @@ addopts = omit = venv/* tests/* + +[pylint.FORMAT] +max-line-length = 120 + +[pylint.MESSAGES CONTROL] +disable=logging-fstring-interpolation + +[pylint.REPORTS] +output-format = colorized diff --git a/src/simplezfs/exceptions.py b/src/simplezfs/exceptions.py index 904620f..68c551b 100644 --- a/src/simplezfs/exceptions.py +++ b/src/simplezfs/exceptions.py @@ -12,49 +12,42 @@ class ZFSException(Exception): :note: Some functions throw ordinary python base exceptions as well. ''' - pass class DatasetNotFound(ZFSException): ''' A dataset was not found. ''' - pass class PermissionError(ZFSException): ''' Permissions are not sufficient to carry out the task. ''' - pass class PoolNotFound(ZFSException): ''' A pool was not found. ''' - pass class PropertyNotFound(ZFSException): ''' A property was not found. ''' - pass class ValidationError(ZFSException): ''' Indicates that a value failed validation. ''' - pass class PEHelperException(ZFSException): ''' Indicates a problem when running the PE helper. ''' - pass class ExternalPEHelperException(PEHelperException): diff --git a/src/simplezfs/pe_helper.py b/src/simplezfs/pe_helper.py index 807de81..c51e83a 100644 --- a/src/simplezfs/pe_helper.py +++ b/src/simplezfs/pe_helper.py @@ -1,10 +1,14 @@ +''' +Privilege escalation helpers +''' + import logging import os import shutil import stat import subprocess -from typing import List, Optional +from typing import List from .exceptions import PEHelperException, ExternalPEHelperException, ValidationError from .validation import validate_dataset_path, validate_pool_name diff --git a/src/simplezfs/types.py b/src/simplezfs/types.py index fe36325..2648580 100644 --- a/src/simplezfs/types.py +++ b/src/simplezfs/types.py @@ -38,14 +38,13 @@ class DatasetType(str, Enum): val = value.lower() if val == 'fileset': return DatasetType.FILESET - elif val == 'volume': + if val == 'volume': return DatasetType.VOLUME - elif val == 'snapshot': + if val == 'snapshot': return DatasetType.SNAPSHOT - elif val == 'bookmark': + if val == 'bookmark': return DatasetType.BOOKMARK - else: - raise ValueError(f'Value {value} is not a valid DatasetType') + raise ValueError(f'Value {value} is not a valid DatasetType') class Dataset(NamedTuple): @@ -128,18 +127,17 @@ class PropertySource(str, Enum): val = value.lower() if val == 'default': return PropertySource.DEFAULT - elif val == 'local': + if val == 'local': return PropertySource.LOCAL - elif val == 'inherited': + if val == 'inherited': return PropertySource.INHERITED - elif val == 'temporary': + if val == 'temporary': return PropertySource.TEMPORARY - elif val == 'received': + if val == 'received': return PropertySource.RECEIVED - elif val == 'none' or val == '-': + if val in ('none', '-'): return PropertySource.NONE - else: - raise ValueError(f'Value {value} is not a valid PropertySource') + raise ValueError(f'Value {value} is not a valid PropertySource') class Property(NamedTuple): @@ -179,7 +177,7 @@ class VDevType(str, Enum): class ZPoolHealth(str, Enum): ''' - + Pool health. ''' ONLINE = 'ONLINE' DEGRADED = 'DEGRADED' @@ -204,17 +202,16 @@ class ZPoolHealth(str, Enum): val = value.lower() if val == 'online': return ZPoolHealth.ONLINE - elif val == 'degraded': + if val == 'degraded': return ZPoolHealth.DEGRADED - elif val == 'faulted': + if val == 'faulted': return ZPoolHealth.FAULTED - elif val == 'offline': + if val == 'offline': return ZPoolHealth.OFFLINE - elif val == 'unavail': + if val == 'unavail': return ZPoolHealth.UNAVAIL - elif val == 'removed': + if val == 'removed': return ZPoolHealth.REMOVED - elif val == 'avail': + if val == 'avail': return ZPoolHealth.AVAIL - else: - raise ValueError(f'Value {value} is not a valid ZPoolHealth') + raise ValueError(f'Value {value} is not a valid ZPoolHealth') diff --git a/src/simplezfs/validation.py b/src/simplezfs/validation.py index 236ef07..77a7e2d 100644 --- a/src/simplezfs/validation.py +++ b/src/simplezfs/validation.py @@ -90,7 +90,7 @@ def validate_dataset_name(name: str, *, strict: bool = False) -> None: match = DATASET_NAME_RE.match(name) if not match: raise ValidationError('name contains disallowed characters') - elif strict and match.group('detail') is not None: + if strict and match.group('detail') is not None: raise ValidationError('snapshot or bookmark identifier are not allowed in strict mode') diff --git a/src/simplezfs/zfs.py b/src/simplezfs/zfs.py index 25efa0a..6f3a4c8 100644 --- a/src/simplezfs/zfs.py +++ b/src/simplezfs/zfs.py @@ -176,7 +176,7 @@ class ZFS: try: self.set_property(fileset, 'mountpoint', mountpoint) - except PermissionError as e: + except PermissionError as exc: if self.pe_helper is not None: real_use_pe_helper = use_pe_helper if use_pe_helper is not None else self.use_pe_helper @@ -185,7 +185,7 @@ class ZFS: self.pe_helper.zfs_set_mountpoint(fileset, mountpoint) else: log.error(f'Permission error when setting mountpoint for "{fileset}" and not using PE helper') - raise e + raise exc else: log.error(f'Permission error when setting mountpoint for "{fileset}" and PE helper is not set') @@ -421,6 +421,7 @@ class ZFS: dataset_type=DatasetType.FILESET, properties=properties, metadata_properties=metadata_properties, + recursive=recursive, ) def create_volume( @@ -537,26 +538,25 @@ class ZFS: # check the syntax of the properties if properties is not None: - for k, v in properties.items(): + for k, val in properties.items(): validate_native_property_name(k) - validate_property_value(v) + validate_property_value(val) _metadata_properties = dict() # type: Dict[str, str] if metadata_properties is not None: - for k, v in metadata_properties.items(): + for k, val in metadata_properties.items(): # if the name has no namespace, add the default one if set if ':' not in k: if not self._metadata_namespace: raise ValidationError(f'Metadata property {k} has no namespace and none is set globally') - else: - meta_name = f'{self._metadata_namespace}:{k}' + meta_name = f'{self._metadata_namespace}:{k}' else: meta_name = k _metadata_properties[meta_name] = metadata_properties[k] validate_metadata_property_name(meta_name) - if type(v) != str: - _metadata_properties[meta_name] = f'{v}' + if type(val) != str: + _metadata_properties[meta_name] = f'{val}' validate_property_value(_metadata_properties[meta_name]) # sparse and size are reset for all but the VOLUME type @@ -584,8 +584,8 @@ class ZFS: raise ValidationError('Size must be specified for volumes') try: size = int(size) - except ValueError as e: - raise ValidationError('Size is not an integer') from e + except ValueError as exc: + raise ValidationError('Size is not an integer') from exc if size < 1: raise ValidationError('Size is too low') @@ -593,8 +593,8 @@ class ZFS: if properties and 'blocksize' in properties: try: blocksize = int(properties['blocksize']) - except ValueError: - raise ValidationError('blocksize must be an integer') + except ValueError as exc: + raise ValidationError('blocksize must be an integer') from exc if blocksize < 2 or blocksize > 128 * 1024: # zfs(8) version 0.8.1 lists 128KB as maximum raise ValidationError('blocksize must be between 2 and 128kb (inclusive)') if not ((blocksize & (blocksize - 1) == 0) and blocksize != 0): @@ -706,7 +706,7 @@ class ZFS: if not self._pe_helper: raise ValidationError('PE Helper is not set') if action not in ('create', 'destroy', 'set_mountpoint'): - raise ValidationError(f'Invalid action') + raise ValidationError('Invalid action') validate_dataset_path(name) if action == 'create': @@ -759,7 +759,7 @@ def get_zfs(api: str = 'cli', metadata_namespace: Optional[str] = None, **kwargs if api == 'cli': from .zfs_cli import ZFSCli return ZFSCli(metadata_namespace=metadata_namespace, **kwargs) - elif api == 'native': + if api == 'native': from .zfs_native import ZFSNative return ZFSNative(metadata_namespace=metadata_namespace, **kwargs) raise NotImplementedError(f'The api "{api}" has not been implemented.') diff --git a/src/simplezfs/zfs_cli.py b/src/simplezfs/zfs_cli.py index 29780c3..e3867cd 100644 --- a/src/simplezfs/zfs_cli.py +++ b/src/simplezfs/zfs_cli.py @@ -138,14 +138,13 @@ class ZFSCli(ZFS): if dataset: raise DatasetNotFound(f'Dataset "{dataset}" not found') raise DatasetNotFound('Dataset not found') - elif 'bad property list: invalid property' in proc.stderr: + if 'bad property list: invalid property' in proc.stderr: if dataset: raise PropertyNotFound(f'invalid property on dataset {dataset}') - else: - raise PropertyNotFound('invalid property') - elif 'permission denied' in proc.stderr: + raise PropertyNotFound('invalid property') + if 'permission denied' in proc.stderr: raise PermissionError(proc.stderr) - elif 'filesystem successfully created, but it may only be mounted by root' in proc.stderr: + if 'filesystem successfully created, but it may only be mounted by root' in proc.stderr: raise PermissionError(proc.stderr) raise Exception(f'Command execution "{" ".join(proc.args)}" failed: {proc.stderr}') @@ -281,10 +280,10 @@ class ZFSCli(ZFS): self.pe_helper.zfs_mount(name) log.info(f'Fileset {name} created successfully (using pe_helper)') return self.get_dataset_info(name) - else: - msg = 'Fileset created partially but no PE helper set' - log.error(msg) - raise PermissionError(msg) + + msg = 'Fileset created partially but no PE helper set' + log.error(msg) + raise PermissionError(msg) else: msg = 'Mountpoint property not set, can\'t run pe_helper' log.error(msg) diff --git a/src/simplezfs/zpool.py b/src/simplezfs/zpool.py index 171f9f8..7da032a 100644 --- a/src/simplezfs/zpool.py +++ b/src/simplezfs/zpool.py @@ -96,7 +96,7 @@ def get_zpool(api: str = 'cli', metadata_namespace: Optional[str] = None, **kwar if api == 'cli': from .zpool_cli import ZPoolCli return ZPoolCli(metadata_namespace=metadata_namespace, **kwargs) - elif api == 'native': + if api == 'native': from .zpool_native import ZPoolNative return ZPoolNative(metadata_namespace=metadata_namespace, **kwargs) raise NotImplementedError(f'The api "{api}" has not been implemented.') diff --git a/src/simplezfs/zpool_cli.py b/src/simplezfs/zpool_cli.py index cea47ff..8e53b2d 100644 --- a/src/simplezfs/zpool_cli.py +++ b/src/simplezfs/zpool_cli.py @@ -14,7 +14,13 @@ log = logging.getLogger('simplezfs.zpool_cli') class ZPoolCli(ZPool): + ''' + ZPOOL interface implementation using the zpool(8) command line utility. For documentation, please see the interface + :class:`~zfs.zpool.ZPool`. It is recommended to use :func:`~zfs.zfs.get_zpool` to obtain an instance using ``cli`` + as api. + If ``zpool_exe`` is supplied, it is assumed that it points to the path to the ``zpool(8)`` executable. + ''' def __init__(self, *, metadata_namespace: Optional[str] = None, pe_helper: Optional[str] = None, use_pe_helper: bool = False, zpool_exe: Optional[str] = None, **kwargs) -> None: super().__init__(metadata_namespace=metadata_namespace, pe_helper=pe_helper, use_pe_helper=use_pe_helper) diff --git a/src/simplezfs/zpool_native.py b/src/simplezfs/zpool_native.py index 3940dcf..0a6c07f 100644 --- a/src/simplezfs/zpool_native.py +++ b/src/simplezfs/zpool_native.py @@ -8,7 +8,11 @@ from .zpool import ZPool class ZPoolNative(ZPool): - + ''' + ZPool interface implementation using the libzfs_core python bindings. For documentation, please see the interface + :class:`~zfs.zpool.ZPool`. It is recommended to use :func:`~zfs.zpool.get_zpool` to obtain an instance, using + ``native`` as api. + ''' def __init__(self, *, metadata_namespace: Optional[str] = None, pe_helper: Optional[str] = None, use_pe_helper: bool = False, **kwargs) -> None: super().__init__(metadata_namespace=metadata_namespace, pe_helper=pe_helper, use_pe_helper=use_pe_helper)