warnings only if needed

diff --git a/test/test_option_consistency.py b/test/test_option_consistency.py
index 01e9c90..dca3e9a 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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):