From b521c459ee58fca4b53e1156b5515e8912194b76 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Sun, 3 Jan 2016 21:18:52 +0100 Subject: [PATCH] remove all try/except --- test/test_config_ip.py | 4 + tiramisu/autolib.py | 37 +++++--- tiramisu/config.py | 85 +++++++++-------- tiramisu/option/baseoption.py | 69 +++++++------- tiramisu/option/masterslave.py | 44 +++++---- tiramisu/option/option.py | 7 +- tiramisu/setting.py | 44 ++++----- tiramisu/storage/dictionary/setting.py | 4 +- tiramisu/value.py | 122 +++++++++++++++---------- 9 files changed, 236 insertions(+), 180 deletions(-) diff --git a/test/test_config_ip.py b/test/test_config_ip.py index c5c61b9..dfcb4af 100644 --- a/test/test_config_ip.py +++ b/test/test_config_ip.py @@ -70,6 +70,10 @@ def test_network(): assert len(w) == 1 +def test_network_invalid(): + raises(ValueError, "NetworkOption('a', '', default='toto')") + + def test_netmask(): a = NetmaskOption('a', '') od = OptionDescription('od', '', [a]) diff --git a/tiramisu/autolib.py b/tiramisu/autolib.py index ace2c0a..8b6de4e 100644 --- a/tiramisu/autolib.py +++ b/tiramisu/autolib.py @@ -25,7 +25,7 @@ from .setting import undefined def carry_out_calculation(option, context, callback, callback_params, - index=undefined): + index=undefined, returns_raise=False): """a function that carries out a calculation for an option's value :param option: the option @@ -160,20 +160,27 @@ def carry_out_calculation(option, context, callback, callback_params, path = context.cfgimpl_get_description( ).impl_get_path_by_opt(opt) # get value - try: - value = context.getattr(path, force_permissive=True, - validate=False) - # convert to list, not modifie this multi - if value.__class__.__name__ == 'Multi': - value = list(value) - except PropertiesOptionError as err: # pragma: optional cover - if force_permissive: - continue - raise ConfigError(_('unable to carry out a calculation' - ', option {0} has properties: {1} ' - 'for: {2}').format(opt.impl_getname(), - err.proptype, - option.impl_getname())) + value = context.getattr(path, force_permissive=True, + validate=False, + returns_raise=True) + if isinstance(value, Exception): + if isinstance(value, PropertiesOptionError): + if force_permissive: + continue + err = ConfigError(_('unable to carry out a calculation' + ', option {0} has properties: {1} ' + 'for: {2}').format(opt.impl_getname(), + value.proptype, + option.impl_getname())) + if returns_raise: + return err + else: + raise err + else: + raise err + # convert to list, not modifie this multi + if value.__class__.__name__ == 'Multi': + value = list(value) if opt.impl_is_master_slaves() and \ opt.impl_get_master_slaves().in_same_group(option): diff --git a/tiramisu/config.py b/tiramisu/config.py index c234717..19b5fe3 100644 --- a/tiramisu/config.py +++ b/tiramisu/config.py @@ -53,7 +53,7 @@ class SubConfig(object): # main option description error = False if descr is not None and not isinstance(descr, OptionDescription) and \ - not isinstance(descr, SynDynOptionDescription): # pragma: optional cover + not isinstance(descr, SynDynOptionDescription): # pragma: optional cover error = True if error: raise TypeError(_('descr must be an optiondescription, not {0}' @@ -242,7 +242,8 @@ class SubConfig(object): return subpath def getattr(self, name, force_permissive=False, validate=True, - _setting_properties=undefined, index=None): + _setting_properties=undefined, index=None, + returns_raise=False): """ attribute notation mechanism for accessing the value of an option :param name: attribute name @@ -257,7 +258,7 @@ class SubConfig(object): return homeconfig.getattr(name, force_permissive=force_permissive, validate=validate, _setting_properties=_setting_properties, - index=index) + index=index, returns_raise=returns_raise) context = self._cfgimpl_get_context() option = self.cfgimpl_get_description().__getattr__(name, context=context) @@ -267,21 +268,25 @@ class SubConfig(object): option, path=subpath, validate=validate, force_permissive=force_permissive, - setting_properties=_setting_properties, index=index) + setting_properties=_setting_properties, index=index, + returns_raise=returns_raise) elif isinstance(option, SymLinkOption): # pragma: no dynoptiondescription cover path = context.cfgimpl_get_description().impl_get_path_by_opt( option._impl_getopt()) return context.getattr(path, validate=validate, force_permissive=force_permissive, _setting_properties=_setting_properties, - index=index) + index=index, returns_raise=returns_raise) elif option.impl_is_optiondescription(): props = self.cfgimpl_get_settings().validate_properties( option, True, False, path=subpath, force_permissive=force_permissive, setting_properties=_setting_properties) if props: - raise props + if returns_raise: + return props + else: + raise props return SubConfig(option, self._impl_context, subpath) else: return self.cfgimpl_get_values()._get_cached_value( @@ -289,7 +294,7 @@ class SubConfig(object): validate=validate, force_permissive=force_permissive, setting_properties=_setting_properties, - index=index) + index=index, returns_raise=returns_raise) def find(self, bytype=None, byname=None, byvalue=undefined, type_='option', check_properties=True, force_permissive=False): @@ -339,15 +344,17 @@ class SubConfig(object): def _filter_by_value(): if byvalue is undefined: return True - try: - value = self.getattr(path, force_permissive=force_permissive, - _setting_properties=setting_properties) - if isinstance(value, Multi): - return byvalue in value - else: - return value == byvalue - except PropertiesOptionError: # pragma: optional cover - return False + value = self.getattr(path, force_permissive=force_permissive, + _setting_properties=setting_properties, + returns_raise=True) + if isinstance(value, Exception): + if isinstance(value, PropertiesOptionError): + return False + raise value + elif isinstance(value, Multi): + return byvalue in value + else: + return value == byvalue if type_ not in ('option', 'path', 'value'): # pragma: optional cover raise ValueError(_('unknown type_ type {0}' @@ -369,12 +376,15 @@ class SubConfig(object): continue #remove option with propertyerror, ... if byvalue is undefined and check_properties: - try: - value = self.getattr(path, - force_permissive=force_permissive, - _setting_properties=setting_properties) - except PropertiesOptionError: # pragma: optional cover - continue + value = self.getattr(path, + force_permissive=force_permissive, + _setting_properties=setting_properties, + returns_raise=True) + if isinstance(value, Exception): + if isinstance(value, PropertiesOptionError): + continue + else: + raise value if type_ == 'value': retval = value elif type_ == 'path': @@ -482,26 +492,25 @@ class SubConfig(object): def _make_sub_dict(self, opt, path, pathsvalues, _currpath, flatten, setting_properties, force_permissive=False): - try: + value = self.getattr(path, + force_permissive=force_permissive, + _setting_properties=setting_properties, + returns_raise=True) + if isinstance(value, Exception): + if not isinstance(value, PropertiesOptionError): + raise value + else: if opt.impl_is_optiondescription(): - pathsvalues += self.getattr(path, - force_permissive=force_permissive, - _setting_properties=setting_properties).make_dict( - flatten, - _currpath + path.split('.'), - force_permissive=force_permissive, - setting_properties=setting_properties) + pathsvalues += value.make_dict(flatten, + _currpath + path.split('.'), + force_permissive=force_permissive, + setting_properties=setting_properties) else: - value = self.getattr(opt.impl_getname(), - force_permissive=force_permissive, - _setting_properties=setting_properties) if flatten: name = opt.impl_getname() else: name = '.'.join(_currpath + [opt.impl_getname()]) pathsvalues.append((name, value)) - except PropertiesOptionError: # pragma: optional cover - pass def cfgimpl_get_path(self, dyn=True): descr = self.cfgimpl_get_description() @@ -790,13 +799,15 @@ class GroupConfig(_CommonConfig): return ret def getattr(self, name, force_permissive=False, validate=True, - _setting_properties=undefined): + _setting_properties=undefined, + returns_raise=False): for child in self._impl_children: if name == child._impl_name: return child return super(GroupConfig, self).getattr(name, force_permissive, validate, - _setting_properties=_setting_properties) + _setting_properties=_setting_properties, + returns_raise=returns_raise) class MetaConfig(GroupConfig): diff --git a/tiramisu/option/baseoption.py b/tiramisu/option/baseoption.py index 09b9034..0c083df 100644 --- a/tiramisu/option/baseoption.py +++ b/tiramisu/option/baseoption.py @@ -387,40 +387,43 @@ class Option(OnlyOption): descr = context.cfgimpl_get_description() all_cons_vals = [] - try: - for opt in all_cons_opts: - #get value - if (isinstance(opt, DynSymLinkOption) and option._dyn == opt._dyn) or \ - option == opt: - opt_value = value - else: - #if context, calculate value, otherwise get default value - if context is not undefined: - if isinstance(opt, DynSymLinkOption): - path = opt.impl_getpath(context) - else: - path = descr.impl_get_path_by_opt(opt) - opt_value = context.getattr(path, validate=False, - force_permissive=True) - else: - opt_value = opt.impl_getdefault() - - #append value - if not self.impl_is_multi() or (isinstance(opt, DynSymLinkOption) - and option._dyn == opt._dyn) or \ - option == opt: - all_cons_vals.append(opt_value) - elif self.impl_is_submulti(): - print option._name, opt_value - all_cons_vals.append(opt_value[index][submulti_index]) - else: - all_cons_vals.append(opt_value[index]) - except PropertiesOptionError as err: - log.debug('propertyerror in _launch_consistency: {0}'.format(err)) - if transitive: - raise err + for opt in all_cons_opts: + #get value + if (isinstance(opt, DynSymLinkOption) and option._dyn == opt._dyn) or \ + option == opt: + opt_value = value else: - return + #if context, calculate value, otherwise get default value + if context is not undefined: + if isinstance(opt, DynSymLinkOption): + path = opt.impl_getpath(context) + else: + path = descr.impl_get_path_by_opt(opt) + opt_value = context.getattr(path, validate=False, + force_permissive=True, + returns_raise=True) + if isinstance(opt_value, Exception): + if isinstance(opt_value, PropertiesOptionError): + log.debug('propertyerror in _launch_consistency: {0}'.format(opt_value)) + if transitive: + raise opt_value + else: + return + else: + raise opt_value + else: + opt_value = opt.impl_getdefault() + + #append value + if not self.impl_is_multi() or (isinstance(opt, DynSymLinkOption) + and option._dyn == opt._dyn) or \ + option == opt: + all_cons_vals.append(opt_value) + #consistency with submulti is now forbidden + #elif self.impl_is_submulti(): + # all_cons_vals.append(opt_value[index][submulti_index]) + else: + all_cons_vals.append(opt_value[index]) return getattr(self, func)(all_cons_opts, all_cons_vals, warnings_only) def impl_validate(self, value, context=undefined, validate=True, diff --git a/tiramisu/option/masterslave.py b/tiramisu/option/masterslave.py index 0a774e5..eff3ce9 100644 --- a/tiramisu/option/masterslave.py +++ b/tiramisu/option/masterslave.py @@ -120,7 +120,8 @@ class MasterSlaves(object): def getitem(self, values, opt, path, validate, force_permissive, trusted_cached_properties, validate_properties, slave_path=undefined, slave_value=undefined, setting_properties=undefined, - self_properties=undefined, index=None): + self_properties=undefined, index=None, + returns_raise=False): if self.is_master(opt): return self._getmaster(values, opt, path, validate, force_permissive, @@ -130,7 +131,7 @@ class MasterSlaves(object): return self._getslave(values, opt, path, validate, force_permissive, trusted_cached_properties, validate_properties, setting_properties, - self_properties, index) + self_properties, index, returns_raise) def _getmaster(self, values, opt, path, validate, force_permissive, validate_properties, c_slave_path, @@ -150,7 +151,7 @@ class MasterSlaves(object): def _getslave(self, values, opt, path, validate, force_permissive, trusted_cached_properties, validate_properties, setting_properties, - self_properties, index): + self_properties, index, returns_raise): """ if master has length 0: return [] @@ -190,7 +191,10 @@ class MasterSlaves(object): force_permissive=force_permissive, setting_properties=setting_properties) if props: - raise props + if returns_raise: + return props + else: + raise props else: one_has_value = False if index is None: @@ -198,22 +202,26 @@ class MasterSlaves(object): else: indexes = [index] for idx in indexes: - try: - value = values._get_cached_value(opt, path, validate, - force_permissive, - trusted_cached_properties, - validate_properties, - with_meta=master_is_meta, - index=idx, - # not self_properties, - # depends to index - #self_properties=self_properties, - masterlen=masterlen, - from_masterslave=True) + value = values._get_cached_value(opt, path, validate, + force_permissive, + trusted_cached_properties, + validate_properties, + with_meta=master_is_meta, + index=idx, + # not self_properties, + # depends to index + #self_properties=self_properties, + masterlen=masterlen, + from_masterslave=True, + returns_raise=True) + if isinstance(value, PropertiesOptionError): + err = value + multi.append_properties_error(value) + elif isinstance(value, Exception): + raise value + else: multi.append(value, setitem=False, force=True, validate=validate) one_has_value = True - except PropertiesOptionError, err: - multi.append_properties_error(err) if not one_has_value: #raise last err raise err diff --git a/tiramisu/option/option.py b/tiramisu/option/option.py index 1ad49d8..6eb3cad 100644 --- a/tiramisu/option/option.py +++ b/tiramisu/option/option.py @@ -232,6 +232,7 @@ class PortOption(Option): see: http://en.wikipedia.org/wiki/Port_numbers """ __slots__ = tuple() + port_re = re.compile(r"^[0-9]*$") def __init__(self, name, doc, default=None, default_multi=None, requires=None, multi=False, callback=None, @@ -262,6 +263,7 @@ class PortOption(Option): if extra['_max_value'] is None: raise ValueError(_('max value is empty')) # pragma: optional cover + super(PortOption, self).__init__(name, doc, default=default, default_multi=default_multi, callback=callback, @@ -292,10 +294,9 @@ class PortOption(Option): value = [value] for val in value: - try: - val = int(val) - except ValueError: # pragma: optional cover + if not self.port_re.search(val): return ValueError(_('invalid port')) + val = int(val) if not self._get_extra('_min_value') <= val <= self._get_extra('_max_value'): # pragma: optional cover return ValueError(_('invalid port, must be an integer between {0} ' 'and {1}').format(self._get_extra('_min_value'), diff --git a/tiramisu/setting.py b/tiramisu/setting.py index 2c02d5f..bfb1fc5 100644 --- a/tiramisu/setting.py +++ b/tiramisu/setting.py @@ -622,28 +622,30 @@ class Settings(object): "imbrication detected for option:" " '{0}' with requirement on: " "'{1}'").format(path, reqpath)) - try: - if option.impl_is_multi(): - idx = index + if option.impl_is_multi(): + idx = index + else: + idx = None + value = context.getattr(reqpath, force_permissive=True, + _setting_properties=setting_properties, + index=idx, returns_raise=True) + if isinstance(value, Exception): + if isinstance(value, PropertiesOptionError): + if not transitive: + continue + properties = value.proptype + if same_action and action not in properties: # pragma: optional cover + raise RequirementError(_("option '{0}' has " + "requirement's property " + "error: " + "{1} {2}").format(opt._name, + reqpath, + properties)) + # transitive action, force expected + value = expected[0] + inverse = False else: - idx = None - value = context.getattr(reqpath, force_permissive=True, - _setting_properties=setting_properties, - index=idx) - except PropertiesOptionError as err: - if not transitive: - continue - properties = err.proptype - if same_action and action not in properties: # pragma: optional cover - raise RequirementError(_("option '{0}' has " - "requirement's property " - "error: " - "{1} {2}").format(opt._name, - reqpath, - properties)) - # transitive action, force expected - value = expected[0] - inverse = False + raise value if (not inverse and value in expected or inverse and value not in expected): diff --git a/tiramisu/storage/dictionary/setting.py b/tiramisu/storage/dictionary/setting.py index 0422314..8c16802 100644 --- a/tiramisu/storage/dictionary/setting.py +++ b/tiramisu/storage/dictionary/setting.py @@ -44,10 +44,8 @@ class Settings(Cache): self._properties.clear() def delproperties(self, path): - try: + if path in self._properties: del(self._properties[path]) - except KeyError: - pass def setpermissive(self, path, permissive): self._permissives[path] = frozenset(permissive) diff --git a/tiramisu/value.py b/tiramisu/value.py index 0fc1ee5..f389837 100644 --- a/tiramisu/value.py +++ b/tiramisu/value.py @@ -57,14 +57,16 @@ class Values(object): def _get_multi(self, opt, path): return Multi([], self.context, opt, path) - def _getdefaultvalue(self, opt, path, with_meta, index, submulti_index): + def _getdefaultvalue(self, opt, path, with_meta, index, submulti_index, + returns_raise): # if value has callback and is not set if opt.impl_has_callback(): callback, callback_params = opt.impl_get_callback() value = carry_out_calculation(opt, context=self._getcontext(), callback=callback, callback_params=callback_params, - index=index) + index=index, + returns_raise=returns_raise) if isinstance(value, list) and index is not None: #if return a list and index is set, return value only if #it's a submulti without submulti_index and without list of list @@ -76,18 +78,19 @@ class Values(object): if with_meta: meta = self._getcontext().cfgimpl_get_meta() if meta is not None: - try: - value = meta.cfgimpl_get_values( - )._get_cached_value(opt, path, index=index, submulti_index=submulti_index, - from_masterslave=True) + value = meta.cfgimpl_get_values( + )._get_cached_value(opt, path, index=index, submulti_index=submulti_index, + from_masterslave=True, returns_raise=True) + if isinstance(value, Exception): + if not isinstance(value, PropertiesOptionError): + raise value + else: if isinstance(value, Multi): if index is not None: value = value[index] else: value = list(value) return value - except PropertiesOptionError: - pass # now try to get default value value = opt.impl_getdefault() if opt.impl_is_multi() and index is not None: @@ -102,7 +105,7 @@ class Values(object): def _getvalue(self, opt, path, is_default, self_properties, index=undefined, submulti_index=undefined, with_meta=True, - masterlen=undefined): + masterlen=undefined, returns_raise=False): """actually retrieves the value :param opt: the `option.Option()` object @@ -123,7 +126,7 @@ class Values(object): #so return default value else: return value - return self._getdefaultvalue(opt, path, with_meta, index, submulti_index) + return self._getdefaultvalue(opt, path, with_meta, index, submulti_index, returns_raise) def get_modified_values(self): context = self._getcontext() @@ -209,7 +212,8 @@ class Values(object): validate_properties=True, setting_properties=undefined, self_properties=undefined, index=None, submulti_index=undefined, from_masterslave=False, - with_meta=True, masterlen=undefined, check_frozen=False): + with_meta=True, masterlen=undefined, check_frozen=False, + returns_raise=False): context = self._getcontext() settings = context.cfgimpl_get_settings() if path is None: @@ -238,7 +242,10 @@ class Values(object): self_properties=self_properties, index=index) if props: - raise props + if returns_raise: + return props + else: + raise props return value if not from_masterslave and opt.impl_is_master_slaves(): val = opt.impl_get_master_slaves().getitem(self, opt, path, @@ -248,7 +255,10 @@ class Values(object): validate_properties, setting_properties=setting_properties, index=index, - self_properties=self_properties) + self_properties=self_properties, + returns_raise=returns_raise) + if isinstance(val, PropertiesOptionError): + return val else: val = self._get_validated_value(opt, path, validate, force_permissive, @@ -259,7 +269,10 @@ class Values(object): masterlen=masterlen, index=index, submulti_index=submulti_index, - check_frozen=check_frozen) + check_frozen=check_frozen, + returns_raise=returns_raise) + if isinstance(val, PropertiesOptionError): + return val # cache doesn't work with SubMulti yet if not isinstance(val, SubMulti) and 'cache' in setting_properties and \ validate and validate_properties and force_permissive is False \ @@ -276,7 +289,7 @@ class Values(object): index=None, submulti_index=undefined, with_meta=True, setting_properties=undefined, self_properties=undefined, masterlen=undefined, - check_frozen=False): + check_frozen=False, returns_raise=False): """same has getitem but don't touch the cache index is None for slave value, if value returned is not a list, just return [] """ @@ -291,25 +304,27 @@ class Values(object): validate_meta=False, self_properties=self_properties, index=index) - try: - value = self._getvalue(opt, path, is_default, self_properties, - index=index, submulti_index=submulti_index, - with_meta=with_meta, - masterlen=masterlen) - config_error = None - except ConfigError as err: - # For calculating properties, we need value (ie for mandatory - # value). - # If value is calculating with a PropertiesOptionError's option - # _getvalue raise a ConfigError. - # We can not raise ConfigError if this option should raise - # PropertiesOptionError too. So we get config_error and raise - # ConfigError if properties did not raise. - # cannot assign config_err directly in python 3.3 - config_error = err - # value is not set, for 'undefined' (cannot set None because of - # mandatory property) - value = undefined + config_error = None + value = self._getvalue(opt, path, is_default, self_properties, + index=index, submulti_index=submulti_index, + with_meta=with_meta, + masterlen=masterlen, + returns_raise=True) + if isinstance(value, Exception): + if isinstance(value, ConfigError): + # For calculating properties, we need value (ie for mandatory + # value). + # If value is calculating with a PropertiesOptionError's option + # _getvalue raise a ConfigError. + # We can not raise ConfigError if this option should raise + # PropertiesOptionError too. So we get config_error and raise + # ConfigError if properties did not raise. + config_error = value + # value is not set, for 'undefined' (cannot set None because of + # mandatory property) + value = undefined + else: + raise value else: if index is undefined: force_index = None @@ -356,9 +371,15 @@ class Values(object): self_properties=self_properties, index=index) if props: - raise props + if returns_raise: + return props + else: + raise props if config_error is not None: - raise config_error + if returns_raise: + return config_error + else: + raise config_error return value def __setitem__(self, opt, value): # pragma: optional cover @@ -591,27 +612,29 @@ class Values(object): else: true_opt = opt true_path = path - #FIXME attention c'est réutilisé donc jamais complet ?? self_properties = settings._getproperties(true_opt, true_path, read_write=False, setting_properties=setting_properties) if 'mandatory' in self_properties: - try: - self._get_cached_value(true_opt, path=true_path, - trusted_cached_properties=False, - force_permissive=force_permissive, - setting_properties=setting_properties, - self_properties=self_properties, - validate=validate) - except PropertiesOptionError as err: + err = self._get_cached_value(true_opt, path=true_path, + trusted_cached_properties=False, + force_permissive=force_permissive, + setting_properties=setting_properties, + self_properties=self_properties, + validate=validate, returns_raise=True) + if not isinstance(err, Exception): + pass + elif isinstance(err, PropertiesOptionError): if err.proptype == ['mandatory']: yield path - except ConfigError as err: + elif isinstance(err, ConfigError): if validate: raise err else: #assume that uncalculated value is an empty value yield path + else: + raise err descr = self._getcontext().cfgimpl_get_description() for path in _mandatory_warnings(descr): @@ -628,10 +651,9 @@ class Values(object): context.cfgimpl_reset_cache() for path in context.cfgimpl_get_description().impl_getpaths( include_groups=True): - try: - context.getattr(path) - except PropertiesOptionError: - pass + err = context.getattr(path, returns_raise=True) + if isinstance(err, Exception) and not isinstance(err, PropertiesOptionError): + raise err def __getstate__(self): return {'_p_': self._p_}