From 0f4b1caca4fa02d0c340f6c36eb4e910e46ae1d4 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Sun, 20 Nov 2016 14:32:06 +0100 Subject: [PATCH] warnings only if needed --- test/test_option_consistency.py | 40 +++++++++ tiramisu/option/baseoption.py | 147 ++++++++++++++++---------------- tiramisu/value.py | 17 +++- 3 files changed, 127 insertions(+), 77 deletions(-) diff --git a/test/test_option_consistency.py b/test/test_option_consistency.py index 01e9c90..dca3e9a 100644 --- a/test/test_option_consistency.py +++ b/test/test_option_consistency.py @@ -703,3 +703,43 @@ def test_consistency_with_callback(): c.impl_add_consistency('in_network', a, b) cfg = Config(od) cfg.c + + +def test_consistency_double_warnings(): + a = IntOption('a', '') + b = IntOption('b', '', 1) + c = IntOption('c', '', 1) + od = OptionDescription('od', '', [a, b, c]) + warnings.simplefilter("always", ValueWarning) + a.impl_add_consistency('not_equal', b, warnings_only=True) + a.impl_add_consistency('not_equal', c, warnings_only=True) + cfg = Config(od) + with warnings.catch_warnings(record=True) as w: + cfg.a = 1 + assert w != [] + assert len(w) == 2 + with warnings.catch_warnings(record=True) as w: + cfg.c = 2 + assert w == [] + with warnings.catch_warnings(record=True) as w: + cfg.a = 2 + assert w != [] + assert len(w) == 1 + cfg.cfgimpl_get_settings().remove('warnings') + with warnings.catch_warnings(record=True) as w: + cfg.a = 1 + assert w == [] + + +def test_consistency_warnings_error(): + a = IntOption('a', '') + b = IntOption('b', '', 1) + c = IntOption('c', '', 1) + od = OptionDescription('od', '', [a, b, c]) + warnings.simplefilter("always", ValueWarning) + a.impl_add_consistency('not_equal', b, warnings_only=True) + a.impl_add_consistency('not_equal', c) + cfg = Config(od) + with warnings.catch_warnings(record=True) as w: + raises(ValueError, "cfg.a = 1") + assert w == [] diff --git a/tiramisu/option/baseoption.py b/tiramisu/option/baseoption.py index c22cd5b..25239d5 100644 --- a/tiramisu/option/baseoption.py +++ b/tiramisu/option/baseoption.py @@ -31,7 +31,6 @@ from ..error import (ConfigError, ValueWarning, PropertiesOptionError, from ..storage import get_storages_option from . import MasterSlaves - if sys.version_info[0] >= 3: # pragma: optional cover xrange = range @@ -469,12 +468,24 @@ class Option(OnlyOption): all_cons_opts.append(opt) if val_consistencies: - return getattr(self, func)(current_opt, all_cons_opts, all_cons_vals, warnings_only) + err = getattr(self, func)(current_opt, all_cons_opts, all_cons_vals, warnings_only) + if err: + if warnings_only: + if isinstance(err, ValueError): + msg = _('attention, "{0}" could be an invalid {1} for "{2}", {3}').format( + value, self._display_name, self.impl_get_display_name(), err) + else: + msg = '{}'.format(err) + warnings.warn_explicit(ValueWarning(msg, self), + ValueWarning, + self.__class__.__name__, 0) + else: + return err def impl_validate(self, value, context=undefined, validate=True, force_index=None, force_submulti_index=None, current_opt=undefined, is_multi=None, - display_warnings=True, multi=None): + display_error=True, display_warnings=True, multi=None): """ :param value: the option's value :param context: Config's context @@ -493,8 +504,10 @@ class Option(OnlyOption): if current_opt is undefined: current_opt = self + display_warnings = display_warnings and (context is undefined or + 'warnings' in context.cfgimpl_get_settings()) def _is_not_unique(value): - if self.impl_is_unique() and len(set(value)) != len(value): + if display_error and self.impl_is_unique() and len(set(value)) != len(value): for idx, val in enumerate(value): if val in value[idx+1:]: return ValueError(_('invalid value "{}", this value is already in "{}"').format( @@ -527,30 +540,27 @@ class Option(OnlyOption): if _value is None: error = warning = None else: - # option validation - err = self._validate(_value, context, current_opt) - if err: - if debug: - log.debug('do_validation: value: {0}, index: {1}, ' - 'submulti_index: {2}'.format(_value, _index, - submulti_index), - exc_info=True) - err_msg = '{0}'.format(err) - name = self.impl_getdoc() - if name is None or name == '': - name = self.impl_getname() - if err_msg: - msg = _('"{0}" is an invalid {1} for "{2}", {3}' - '').format(_value, self._display_name, - self.impl_get_display_name(), err_msg) - else: - msg = _('"{0}" is an invalid {1} for "{2}"' - '').format(_value, self._display_name, - self.impl_get_display_name()) - return ValueError(msg) - warning = None + if display_error: + # option validation + err = self._validate(_value, context, current_opt) + if err: + if debug: + log.debug('do_validation: value: {0}, index: {1}, ' + 'submulti_index: {2}'.format(_value, _index, + submulti_index), + exc_info=True) + err_msg = '{0}'.format(err) + if err_msg: + msg = _('"{0}" is an invalid {1} for "{2}", {3}' + '').format(_value, self._display_name, + self.impl_get_display_name(), err_msg) + else: + msg = _('"{0}" is an invalid {1} for "{2}"' + '').format(_value, self._display_name, + self.impl_get_display_name()) + return ValueError(msg) error = None - if display_warnings: + if (display_error and not self._is_warnings_only()) or (display_warnings and self._is_warnings_only()): error = calculation_validator(_value) if not error: error = self._second_level_validation(_value, self._is_warnings_only()) @@ -559,30 +569,23 @@ class Option(OnlyOption): log.debug(_('do_validation for {0}: error in value').format( self.impl_getname()), exc_info=True) if self._is_warnings_only(): - warning = error + msg = _('attention, "{0}" could be an invalid {1} for "{2}", {3}').format( + _value, self._display_name, self.impl_get_display_name(), error) + warnings.warn_explicit(ValueWarning(msg, self), + ValueWarning, + self.__class__.__name__, 0) error = None - if error is None and warning is None: + if error is None: # if context launch consistency validation #if context is not undefined: ret = self._valid_consistency(current_opt, _value, context, - _index, submulti_index) - if ret: - if isinstance(ret, ValueWarning): - if display_warnings: - warning = ret - elif isinstance(ret, ValueError): - error = ret - else: - return ret - if warning: - msg = _('attention, "{0}" could be an invalid {1} for "{2}", {3}').format( - _value, self._display_name, self.impl_get_display_name(), warning) - if context is undefined or 'warnings' in \ - context.cfgimpl_get_settings(): - warnings.warn_explicit(ValueWarning(msg, self), - ValueWarning, - self.__class__.__name__, 0) - elif error: + _index, submulti_index, display_warnings, + display_error) + if isinstance(ret, ValueError): + error = ret + elif ret: + return ret + if error: err_msg = '{0}'.format(error) if err_msg: msg = _('"{0}" is an invalid {1} for "{2}", {3}' @@ -594,10 +597,6 @@ class Option(OnlyOption): self.impl_get_display_name()) return ValueError(msg) - # generic calculation - #if context is not undefined: - # descr = context.cfgimpl_get_description() - if is_multi is None: is_multi = self.impl_is_multi() @@ -653,7 +652,7 @@ class Option(OnlyOption): if err: return err return self._valid_consistency(current_opt, None, context, - None, None) + None, None, display_warnings, display_error) def impl_is_dynsymlinkoption(self): return False @@ -752,7 +751,8 @@ class Option(OnlyOption): #consistency could generate warnings or errors self._set_has_dependency() - def _valid_consistency(self, option, value, context, index, submulti_idx): + def _valid_consistency(self, option, value, context, index, submulti_idx, + display_warnings, display_error): if context is not undefined: descr = context.cfgimpl_get_description() if descr._cache_consistencies is None: @@ -767,27 +767,25 @@ class Option(OnlyOption): if consistencies is not None: for func, all_cons_opts, params in consistencies: warnings_only = params.get('warnings_only', False) - transitive = params.get('transitive', True) - #all_cons_opts[0] is the option where func is set - if isinstance(option, DynSymLinkOption): - subpath = '.'.join(option._dyn.split('.')[:-1]) - namelen = len(option._impl_getopt().impl_getname()) - suffix = option.impl_getname()[namelen:] - opts = [] - for opt in all_cons_opts: - name = opt.impl_getname() + suffix - path = subpath + '.' + name - opts.append(opt._impl_to_dyn(name, path)) - else: - opts = all_cons_opts - err = opts[0]._launch_consistency(self, func, option, value, - context, index, submulti_idx, - opts, warnings_only, - transitive) - if err: - if warnings_only: - return ValueWarning(str(err), option) + if (warnings_only and display_warnings) or (not warnings_only and display_error): + transitive = params.get('transitive', True) + #all_cons_opts[0] is the option where func is set + if isinstance(option, DynSymLinkOption): + subpath = '.'.join(option._dyn.split('.')[:-1]) + namelen = len(option._impl_getopt().impl_getname()) + suffix = option.impl_getname()[namelen:] + opts = [] + for opt in all_cons_opts: + name = opt.impl_getname() + suffix + path = subpath + '.' + name + opts.append(opt._impl_to_dyn(name, path)) else: + opts = all_cons_opts + err = opts[0]._launch_consistency(self, func, option, value, + context, index, submulti_idx, + opts, warnings_only, + transitive) + if err: return err def _cons_not_equal(self, current_opt, opts, vals, warnings_only): @@ -1062,12 +1060,13 @@ class DynSymLinkOption(object): def impl_validate(self, value, context=undefined, validate=True, force_index=None, force_submulti_index=None, is_multi=None, - display_warnings=True, multi=None): + display_error=True, display_warnings=True, multi=None): return self._impl_getopt().impl_validate(value, context, validate, force_index, force_submulti_index, current_opt=self, is_multi=is_multi, + display_error=display_error, display_warnings=display_warnings, multi=multi) diff --git a/tiramisu/value.py b/tiramisu/value.py index 49d18de..df93256 100644 --- a/tiramisu/value.py +++ b/tiramisu/value.py @@ -213,7 +213,7 @@ class Values(object): def __getitem__(self, opt): "enables us to use the pythonic dictionary-like access to values" - return self.getitem(opt) + return self._get_cached_value(opt) def getitem(self, opt, validate=True, force_permissive=False): """ @@ -315,6 +315,7 @@ class Values(object): value = self._getvalue(opt, path, self_properties, index, submulti_index, with_meta, masterlen, session) if isinstance(value, Exception): + value_error = True if isinstance(value, ConfigError): # For calculating properties, we need value (ie for mandatory # value). @@ -330,6 +331,7 @@ class Values(object): else: raise value else: + value_error = False if index is undefined: force_index = None else: @@ -350,7 +352,8 @@ class Values(object): 'validator' in setting_properties, force_index=force_index, force_submulti_index=force_submulti_index, - display_warnings=display_warnings) + display_error=True, + display_warnings=False) if err: config_error = err value = None @@ -370,6 +373,13 @@ class Values(object): index=index) if props: return props + if not value_error and validate and display_warnings: + opt.impl_validate(value, context, + 'validator' in setting_properties, + force_index=force_index, + force_submulti_index=force_submulti_index, + display_error=False, + display_warnings=display_warnings) if config_error is not None: return config_error return value @@ -396,9 +406,10 @@ class Values(object): session=session, not_raises=not_raises) if props and not_raises: return - err = opt.impl_validate(value, fake_context) + err = opt.impl_validate(value, fake_context, display_warnings=False) if err: raise err + opt.impl_validate(value, fake_context, display_error=False) self._setvalue(opt, path, value) def _setvalue(self, opt, path, value, force_owner=undefined, index=None):