openstack/diskimage-builder
Code Issues Proposed changes

Remove _config_error thrower

Browse Source 
"log and throw" is arguably an anti-pattern; the error message either
bubles-up into the exception, or the handler figures it out.  We have
an example where this logs, and then the handler in blockdevice.py
catches it and logs it again.

Less layers is better; just raise the exception, and use log.exception
to get tracebacks where handled.

Change-Id: I8efd94fbe52a3911253753f447afdb7565849185
This commit is contained in:
Ian Wienand 2017-05-10 09:59:26 +10:00
parent c54e9fddfa
commit b91207ae47
4 changed files with 35 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
diskimage_builder/block_device/level1/partitioning.py
View File

@ -152,14 +152,19 @@ class Partitioning(PluginBase):
# Parameter check # Parameter check
if 'base' not in config: if 'base' not in config:
self._config_error("Partitioning config needs 'base'") raise BlockDeviceSetupException("Partitioning config needs 'base'")
self.base = config['base'] self.base = config['base']
if 'partitions' not in config:
raise BlockDeviceSetupException(
"Partitioning config needs 'partitions'")
if 'label' not in config: if 'label' not in config:
self._config_error("Partitioning config needs 'label'") raise BlockDeviceSetupException(
"Partitioning config needs 'label'")
self.label = config['label'] self.label = config['label']
if self.label not in ("mbr", ): if self.label not in ("mbr", ):
self._config_error("Label must be 'mbr'") raise BlockDeviceSetupException("Label must be 'mbr'")
# It is VERY important to get the alignment correct. If this # It is VERY important to get the alignment correct. If this
# is not correct, the disk performance might be very poor. # is not correct, the disk performance might be very poor.
@ -176,15 +181,13 @@ class Partitioning(PluginBase):
if 'align' in config: if 'align' in config:
self.align = parse_abs_size_spec(config['align']) self.align = parse_abs_size_spec(config['align'])
if 'partitions' not in config:
self._config_error("Partitioning config needs 'partitions'")
self.partitions = [] self.partitions = []
prev_partition = None prev_partition = None
for part_cfg in config['partitions']: for part_cfg in config['partitions']:
if 'name' not in part_cfg: if 'name' not in part_cfg:
self.config_error("Missing 'name' in partition config") raise BlockDeviceSetupException(
"Missing 'name' in partition config")
part_name = part_cfg['name'] part_name = part_cfg['name']
flags = set() flags = set()
@ -195,11 +198,12 @@ class Partitioning(PluginBase):
elif f == 'primary': elif f == 'primary':
flags.add(Partitioning.flag_primary) flags.add(Partitioning.flag_primary)
else: else:
self._config_error("Unknown flag [%s] in " raise BlockDeviceSetupException(
"partitioning for [%s]" "Unknown flag [%s] in partitioning for [%s]"
% (f, part_name)) % (f, part_name))
if 'size' not in part_cfg: if 'size' not in part_cfg:
self._config_error("No 'size' in partition [%s]" raise BlockDeviceSetupException("No 'size' in partition [%s]"
% part_name) % part_name)
size = part_cfg['size'] size = part_cfg['size']
@ -211,10 +215,6 @@ class Partitioning(PluginBase):
prev_partition = np prev_partition = np
logger.debug(part_cfg) logger.debug(part_cfg)
def _config_error(self, msg):
logger.error(msg)
raise BlockDeviceSetupException(msg)
def _size_of_block_dev(self, dev): def _size_of_block_dev(self, dev):
with open(dev, "r") as fd: with open(dev, "r") as fd:
fd.seek(0, 2) fd.seek(0, 2)

28
diskimage_builder/block_device/level2/mkfs.py
View File

@ -43,16 +43,13 @@ file_system_max_label_length = {
class Filesystem(Digraph.Node): class Filesystem(Digraph.Node):
def _config_error(self, msg):
logger.error(msg)
raise BlockDeviceSetupException(msg)
def __init__(self, config): def __init__(self, config):
logger.debug("Create filesystem object; config [%s]" % config) logger.debug("Create filesystem object; config [%s]" % config)
# Parameter check (mandatory) # Parameter check (mandatory)
for pname in ['base', 'name', 'type']: for pname in ['base', 'name', 'type']:
if pname not in config: if pname not in config:
self._config_error("Mkfs config needs [%s]" % pname) raise BlockDeviceSetupException(
"Mkfs config needs [%s]" % pname)
setattr(self, pname, config[pname]) setattr(self, pname, config[pname])
# Parameter check (optional) # Parameter check (optional)
@ -71,20 +68,19 @@ class Filesystem(Digraph.Node):
self.label = "img-rootfs" self.label = "img-rootfs"
if self.label in file_system_labels: if self.label in file_system_labels:
self._config_error( raise BlockDeviceSetupException(
"File system label [%s] used more than once" % "File system label [%s] used more than once" % self.label)
self.label)
file_system_labels.add(self.label) file_system_labels.add(self.label)
if self.type in file_system_max_label_length: if self.type in file_system_max_label_length:
if file_system_max_label_length[self.type] < \ if file_system_max_label_length[self.type] < len(self.label):
len(self.label): raise BlockDeviceSetupException(
self._config_error( "Label [{label}] too long for filesystem [{type}]: "
"Label [%s] too long for filesystem [%s]: " " [{len}] > [{max_len}]".format({
"maximum length [%d] provided length [%d]" % 'label': self.label,
(self.label, self.type, 'type': self.type,
file_system_max_label_length[self.type], 'len': len(self.label),
len(self.label))) 'max': file_system_max_label_length[self.type]}))
else: else:
logger.warning("Length of label [%s] cannot be checked for " logger.warning("Length of label [%s] cannot be checked for "
"filesystem [%s]: unknown max length" % "filesystem [%s]: unknown max length" %

18
diskimage_builder/block_device/level3/mount.py
View File

@ -35,17 +35,13 @@ sorted_mount_points = None
class MountPoint(Digraph.Node): class MountPoint(Digraph.Node):
@staticmethod
def _config_error(msg):
logger.error(msg)
raise BlockDeviceSetupException(msg)
def __init__(self, mount_base, config): def __init__(self, mount_base, config):
# Parameter check # Parameter check
self.mount_base = mount_base self.mount_base = mount_base
for pname in ['base', 'name', 'mount_point']: for pname in ['base', 'name', 'mount_point']:
if pname not in config: if pname not in config:
self._config_error("MountPoint config needs [%s]" % pname) raise BlockDeviceSetupException(
"MountPoint config needs [%s]" % pname)
setattr(self, pname, config[pname]) setattr(self, pname, config[pname])
Digraph.Node.__init__(self, self.name) Digraph.Node.__init__(self, self.name)
logger.debug("MountPoint created [%s]" % self) logger.debug("MountPoint created [%s]" % self)
@ -57,7 +53,8 @@ class MountPoint(Digraph.Node):
def insert_node(self, dg): def insert_node(self, dg):
global mount_points global mount_points
if self.mount_point in mount_points: if self.mount_point in mount_points:
self._config_error("Mount point [%s] specified more than once" raise BlockDeviceSetupException(
"Mount point [%s] specified more than once"
% self.mount_point) % self.mount_point)
logger.debug("Insert node [%s]" % self) logger.debug("Insert node [%s]" % self)
mount_points[self.mount_point] = self mount_points[self.mount_point] = self
@ -136,17 +133,14 @@ class Mount(object):
type_string = "mount" type_string = "mount"
tree_config = TreeConfig("mount") tree_config = TreeConfig("mount")
def _config_error(self, msg):
logger.error(msg)
raise BlockDeviceSetupException(msg)
def __init__(self, config, params): def __init__(self, config, params):
logger.debug("Mounting object; config [%s]" % config) logger.debug("Mounting object; config [%s]" % config)
self.config = config self.config = config
self.params = params self.params = params
if 'mount-base' not in self.params: if 'mount-base' not in self.params:
MountPoint._config_error("Mount default config needs 'mount-base'") raise BlockDeviceSetupException(
"Mount default config needs 'mount-base'")
self.mount_base = self.params['mount-base'] self.mount_base = self.params['mount-base']
self.mount_points = {} self.mount_points = {}

6
diskimage_builder/block_device/level4/fstab.py
View File

@ -14,8 +14,6 @@
import logging import logging
from diskimage_builder.block_device.blockdevice \
import BlockDeviceSetupException
from diskimage_builder.block_device.tree_config import TreeConfig from diskimage_builder.block_device.tree_config import TreeConfig
from diskimage_builder.graph.digraph import Digraph from diskimage_builder.graph.digraph import Digraph
@ -28,10 +26,6 @@ class Fstab(Digraph.Node):
type_string = "fstab" type_string = "fstab"
tree_config = TreeConfig("fstab") tree_config = TreeConfig("fstab")
def _config_error(self, msg):
logger.error(msg)
raise BlockDeviceSetupException(msg)
def __init__(self, config, params): def __init__(self, config, params):
logger.debug("Fstab object; config [%s]" % config) logger.debug("Fstab object; config [%s]" % config)
self.config = config self.config = config