introduce pylint and address some warnings

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

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):

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

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')

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')
 
 

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:
+                if type(val) != str:
-                    _metadata_properties[meta_name] = f'{v}'
+                    _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:
+                except ValueError as exc:
-                    raise ValidationError('Size is not an integer') from e
+                    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:
+                    except ValueError as exc:
-                        raise ValidationError('blocksize must be an integer')
+                        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.')

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')
-                raise PropertyNotFound('invalid property')
+        if 'permission denied' in proc.stderr:
-        elif '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'
+                            msg = 'Fileset created partially but no PE helper set'
-                                log.error(msg)
+                            log.error(msg)
-                                raise PermissionError(msg)
+                            raise PermissionError(msg)
                         else:
                             msg = 'Mountpoint property not set, can\'t run pe_helper'
                             log.error(msg)

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.')

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)

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)