user friendly reporting of config errors

Config errors are collected first, and raised after processing
all modules. This is more user friendly.

+ remove redundant check for predefined accessibles in modules.py
+ fixed error handling for exporting parameters in params.py
+ fixed handling of bare attributes overwriting properties

Change-Id: I894bda291ab85ccec3d771c4903393c808af0a2a
This commit is contained in:
zolliker 2021-03-08 10:27:43 +01:00
parent bd14e0a0e5
commit 3dc159a9a4
4 changed files with 106 additions and 76 deletions

View File

@ -30,9 +30,9 @@ from secop.datatypes import ArrayOf, BoolType, EnumType, FloatRange, \
IntRange, StatusType, StringType, TextType, TupleOf, get_datatype
from secop.errors import BadValueError, ConfigError, InternalError, \
ProgrammingError, SECoPError, SilentError, secop_error
from secop.lib import formatException, formatExtendedStack, mkthread
from secop.lib import formatException, mkthread
from secop.lib.enum import Enum
from secop.params import PREDEFINED_ACCESSIBLES, Accessible, Command, Parameter
from secop.params import Accessible, Command, Parameter
from secop.poller import BasicPoller, Poller
from secop.properties import HasProperties, Property
@ -237,29 +237,27 @@ class Module(HasAccessibles):
self.name = name
self.valueCallbacks = {}
self.errorCallbacks = {}
errors = []
# handle module properties
# 1) make local copies of properties
super().__init__()
# 2) check and apply properties specified in cfgdict
# specified as '.<propertyname> = <propertyvalue>'
# (this is for legacy config files only)
for k, v in list(cfgdict.items()): # keep list() as dict may change during iter
if k[0] == '.':
if k[1:] in self.propertyDict:
self.setProperty(k[1:], cfgdict.pop(k))
else:
raise ConfigError('Module %r has no property %r' %
(self.name, k[1:]))
# 2) check and apply properties specified in cfgdict as
# '<propertyname> = <propertyvalue>'
for key in self.propertyDict:
value = cfgdict.pop(key, None)
if value is None:
# legacy cfg: specified as '.<propertyname> = <propertyvalue>'
value = cfgdict.pop('.' + key, None)
if value is not None:
try:
self.setProperty(key, value)
except BadValueError:
errors.append('module %s, %s: value %r does not match %r!' %
(name, key, value, self.propertyDict[key].datatype))
# 3) check and apply properties specified in cfgdict as
# '<propertyname> = <propertyvalue>' (without '.' prefix)
for k in self.propertyDict:
if k in cfgdict:
self.setProperty(k, cfgdict.pop(k))
# 4) set automatic properties
# 3) set automatic properties
mycls = self.__class__
myclassname = '%s.%s' % (mycls.__module__, mycls.__name__)
self.implementation = myclassname
@ -293,16 +291,6 @@ class Module(HasAccessibles):
if not self.export: # do not export parameters of a module not exported
aobj.export = False
if aobj.export:
if aobj.export is True:
predefined_obj = PREDEFINED_ACCESSIBLES.get(aname, None)
if predefined_obj:
if isinstance(aobj, predefined_obj):
aobj.export = aname
else:
raise ProgrammingError("can not use '%s' as name of a %s" %
(aname, aobj.__class__.__name__))
else: # create custom parameter
aobj.export = '_' + aname
accessiblename2attr[aobj.export] = aname
accessibles[aname] = aobj
# do not re-use self.accessibles as this is the same for all instances
@ -317,29 +305,31 @@ class Module(HasAccessibles):
for k, v in list(cfgdict.items()): # keep list() as dict may change during iter
if '.' in k[1:]:
paramname, propname = k.split('.', 1)
propvalue = cfgdict.pop(k)
paramobj = self.accessibles.get(paramname, None)
# paramobj might also be a command (not sure if this is needed)
if paramobj:
if propname == 'datatype':
paramobj.setProperty('datatype', get_datatype(cfgdict.pop(k), k))
elif propname in paramobj.getProperties():
paramobj.setProperty(propname, cfgdict.pop(k))
else:
raise ConfigError('Module %s: Parameter %r has no property %r!' %
(self.name, paramname, propname))
propvalue = get_datatype(propvalue, k)
try:
paramobj.setProperty(propname, propvalue)
except KeyError:
errors.append('module %s: %s.%s does not exist' %
(self.name, paramname, propname))
except BadValueError as e:
errors.append('module %s: %s.%s: %s' %
(self.name, paramname, propname, str(e)))
else:
raise ConfigError('Module %s has no Parameter %r!' %
(self.name, paramname))
errors.append('module %s: %s not found' % (self.name, paramname))
# 3) check config for problems:
# only accept remaining config items specified in parameters
for k, v in cfgdict.items():
if k not in self.parameters:
raise ConfigError(
'Module %s:config Parameter %r '
'not understood! (use one of %s)' %
(self.name, k, ', '.join(list(self.parameters) +
list(self.propertyDict))))
bad = [k for k in cfgdict if k not in self.parameters]
if bad:
errors.append(
'module %s: %s does not exist (use one of %s)' %
(self.name, ', '.join(bad), ', '.join(list(self.parameters) +
list(self.propertyDict))))
# 4) complain if a Parameter entry has no default value and
# is not specified in cfgdict and deal with parameters to be written.
@ -354,15 +344,15 @@ class Module(HasAccessibles):
# TODO: not sure about readonly (why not a parameter which can only be written from config?)
try:
pobj.value = pobj.datatype(cfgdict[pname])
self.writeDict[pname] = pobj.value
except BadValueError as e:
raise ConfigError('%s.%s: %s' % (name, pname, e))
self.writeDict[pname] = pobj.value
errors.append('module %s, parameter %s: %s' % (name, pname, e))
else:
if pobj.default is None:
if pobj.needscfg:
raise ConfigError('Parameter %s.%s has no default '
'value and was not given in config!' %
(self.name, pname))
errors.append('module %s, parameter %s has no default '
'value and was not given in config!' %
(self.name, pname))
# we do not want to call the setter for this parameter for now,
# this should happen on the first read
pobj.readerror = ConfigError('not initialized')
@ -373,8 +363,8 @@ class Module(HasAccessibles):
try:
value = pobj.datatype(pobj.default)
except BadValueError as e:
raise ProgrammingError('bad default for %s.%s: %s'
% (name, pname, e))
# this should not happen, as the default is already checked in Parameter
raise ProgrammingError('bad default for %s:%s: %s' % (name, pname, e)) from None
if pobj.initwrite and not pobj.readonly:
# we will need to call write_<pname>
# if this is not desired, the default must not be given
@ -392,10 +382,8 @@ class Module(HasAccessibles):
# note: this will NOT call write_* methods!
setattr(self, k, v)
except (ValueError, TypeError):
self.log.exception(formatExtendedStack())
raise
# raise ConfigError('Module %s: config parameter %r:\n%r' %
# (self.name, k, e))
# self.log.exception(formatExtendedStack())
errors.append('module %s, parameter %s: %s' % (self.name, k, e))
cfgdict.pop(k)
# Modify units AFTER applying the cfgdict
@ -405,9 +393,18 @@ class Module(HasAccessibles):
dt.setProperty('unit', dt.unit.replace('$', self.parameters['value'].datatype.unit))
# 6) check complete configuration of * properties
self.checkProperties()
for p in self.parameters.values():
p.checkProperties()
if not errors:
try:
self.checkProperties()
except ConfigError as e:
errors.append('module %s: %s' % (name, e))
for pname, p in self.parameters.items():
try:
p.checkProperties()
except ConfigError:
errors.append('module %s, parameter %s: %s' % (name, pname, e))
if errors:
raise ConfigError('\n'.join(errors))
# helper cfg-editor
def __iter__(self):

View File

@ -225,10 +225,13 @@ class Parameter(Accessible):
self.propertyValues.pop('default')
if self.export is True:
if isinstance(self, PREDEFINED_ACCESSIBLES.get(name, type(None))):
predefined_cls = PREDEFINED_ACCESSIBLES.get(name, None)
if predefined_cls is Parameter:
self.export = name
else:
elif predefined_cls is None:
self.export = '_' + name
else:
raise ProgrammingError('can not use %r as name of a Parameter' % name)
def copy(self):
# deep copy, as datatype might be altered from config
@ -339,10 +342,13 @@ class Command(Accessible):
self.datatype = CommandType(self.argument, self.result)
if self.export is True:
if isinstance(self, PREDEFINED_ACCESSIBLES.get(name, type(None))):
predefined_cls = PREDEFINED_ACCESSIBLES.get(name, None)
if predefined_cls is Command:
self.export = name
else:
elif predefined_cls is None:
self.export = '_' + name
else:
raise ProgrammingError('can not use %r as name of a Command' % name)
def __get__(self, obj, owner=None):
if obj is None:

View File

@ -158,7 +158,7 @@ class HasProperties(HasDescriptors):
cls.propertyDict = properties
# treat overriding properties with bare values
for pn, po in properties.items():
value = cls.__dict__.get(pn, po)
value = getattr(cls, pn, po)
if not isinstance(value, Property): # attribute is a bare value
po = Property(**po.__dict__)
try:
@ -177,11 +177,11 @@ class HasProperties(HasDescriptors):
"""validates properties and checks for min... <= max..."""
for pn, po in self.propertyDict.items():
if po.mandatory:
if pn not in self.propertyDict:
try:
self.propertyValues[pn] = po.datatype(self.propertyValues[pn])
except (KeyError, BadValueError) as e:
name = getattr(self, 'name', self.__class__.__name__)
raise ConfigError('Property %r of %s needs a value of type %r!' % (pn, name, po.datatype))
# apply validator (which may complain further)
self.propertyValues[pn] = po.datatype(self.propertyValues[pn])
raise ConfigError('%s.%s needs a value of type %r!' % (name, pn, po.datatype))
for pn, po in self.propertyDict.items():
if pn.startswith('min'):
maxname = 'max' + pn[3:]

View File

@ -26,11 +26,12 @@
import ast
import configparser
import os
import sys
import threading
import time
from collections import OrderedDict
from secop.errors import ConfigError
from secop.errors import ConfigError, SECoPError
from secop.lib import formatException, get_class, getGeneralConfig
from secop.modules import Attached
from secop.params import PREDEFINED_ACCESSIBLES
@ -179,8 +180,8 @@ class Server:
self.run()
def unknown_options(self, cls, options):
raise ConfigError("%s class don't know how to handle option(s): %s" %
(cls.__name__, ', '.join(options)))
return ("%s class don't know how to handle option(s): %s" %
(cls.__name__, ', '.join(options)))
def run(self):
while self._restart:
@ -201,7 +202,7 @@ class Server:
cls = get_class(self.INTERFACES[scheme])
with cls(scheme, self.log.getChild(scheme), opts, self) as self.interface:
if opts:
self.unknown_options(cls, opts)
raise ConfigError(self.unknown_options(cls, opts))
self.log.info('startup done, handling transport messages')
if systemd:
systemd.daemon.notify("READY=1\nSTATUS=accepting requests")
@ -219,16 +220,26 @@ class Server:
self.interface.shutdown()
def _processCfg(self):
errors = []
opts = dict(self.node_cfg)
cls = get_class(opts.pop('class', 'protocol.dispatcher.Dispatcher'))
self.dispatcher = cls(opts.pop('name', self._cfgfiles), self.log.getChild('dispatcher'), opts, self)
if opts:
self.unknown_options(cls, opts)
errors.append(self.unknown_options(cls, opts))
self.modules = OrderedDict()
badclass = None
self.lastError = None
for modname, options in self.module_cfg.items():
opts = dict(options)
cls = get_class(opts.pop('class'))
modobj = cls(modname, self.log.getChild(modname), opts, self)
try:
classname = opts.pop('class')
cls = get_class(classname)
modobj = cls(modname, self.log.getChild(modname), opts, self)
except ConfigError as e:
errors.append(str(e))
except Exception as e:
badclass = classname
errors.append('error while loading %s' % badclass)
# all used args should be popped from opts!
if opts:
self.unknown_options(cls, opts)
@ -249,12 +260,28 @@ class Server:
for modname, modobj in self.modules.items():
for propname, propobj in modobj.propertyDict.items():
if isinstance(propobj, Attached):
setattr(modobj, propobj.attrname or '_' + propname,
self.dispatcher.get_module(getattr(modobj, propname)))
try:
setattr(modobj, propobj.attrname or '_' + propname,
self.dispatcher.get_module(getattr(modobj, propname)))
except SECoPError as e:
errors.append('module %s, attached %s: %s' % (modname, propname, str(e)))
# call init on each module after registering all
for modname, modobj in self.modules.items():
modobj.initModule()
if errors:
for errtxt in errors:
for line in errtxt.split('\n'):
self.log.error(line)
# print a list of config errors to stderr
sys.stderr.write('\n'.join(errors))
sys.stderr.write('\n')
if badclass:
# force stack trace for import of last erroneous module
get_class(badclass)
sys.exit(1)
if self._testonly:
return
start_events = []