From f4677b9ef94a82e2c8f19fbcbebee251183cbe0f Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Thu, 26 Sep 2013 21:56:06 +0200 Subject: [PATCH] use warnings instead of a new dictionary --- test/test_option_validator.py | 98 +++++++++++++++++++++++------------ tiramisu/error.py | 32 ++++++++++++ tiramisu/option.py | 9 ++-- tiramisu/value.py | 42 ++++----------- 4 files changed, 112 insertions(+), 69 deletions(-) diff --git a/test/test_option_validator.py b/test/test_option_validator.py index 057aa4d..76078f1 100644 --- a/test/test_option_validator.py +++ b/test/test_option_validator.py @@ -1,9 +1,11 @@ import autopath +import warnings from py.test import raises from tiramisu.config import Config from tiramisu.option import StrOption, OptionDescription from tiramisu.setting import groups +from tiramisu.error import ValueWarning def return_true(value, param=None): @@ -76,24 +78,36 @@ def test_validator_warning(): root = OptionDescription('root', '', [opt1, opt2, opt3]) cfg = Config(root) assert cfg.opt1 == 'val' - cfg.opt1 = 'val' - assert cfg.cfgimpl_get_values().has_warning() is False - cfg.opt2 = 'val' - assert cfg.cfgimpl_get_values().has_warning() is True - assert cfg.cfgimpl_get_values().get_warnings() == {opt2: 'invalid value val for option opt2: error'} - assert cfg.cfgimpl_get_values().has_warning() is False - cfg.opt3.append('val') - assert cfg.cfgimpl_get_values().has_warning() is False - cfg.opt3.append('val1') - assert cfg.cfgimpl_get_values().has_warning() is True - assert cfg.cfgimpl_get_values().get_warnings() == {opt3: 'invalid value val1 for option opt3: error'} - assert cfg.cfgimpl_get_values().has_warning() is False + warnings.simplefilter("always", ValueWarning) + with warnings.catch_warnings(record=True) as w: + cfg.opt1 = 'val' + assert w == [] + # + with warnings.catch_warnings(record=True) as w: + cfg.opt2 = 'val' + assert len(w) == 1 + assert w[0].message.opt == opt2 + assert str(w[0].message) == 'invalid value val for option opt2: error' + # + with warnings.catch_warnings(record=True) as w: + cfg.opt3.append('val') + assert w == [] + # + with warnings.catch_warnings(record=True) as w: + cfg.opt3.append('val1') + assert len(w) == 1 + assert w[0].message.opt == opt3 + assert str(w[0].message) == 'invalid value val1 for option opt3: error' raises(ValueError, "cfg.opt2 = 1") - cfg.opt2 = 'val' - cfg.opt3.append('val') - assert cfg.cfgimpl_get_values().has_warning() is True - assert cfg.cfgimpl_get_values().get_warnings() == {opt2: 'invalid value val for option opt2: error', opt3: 'invalid value val1 for option opt3: error'} - assert cfg.cfgimpl_get_values().has_warning() is False + # + with warnings.catch_warnings(record=True) as w: + cfg.opt2 = 'val' + cfg.opt3.append('val') + assert len(w) == 2 + assert w[0].message.opt == opt2 + assert str(w[0].message) == 'invalid value val for option opt2: error' + assert w[1].message.opt == opt3 + assert str(w[1].message) == 'invalid value val1 for option opt3: error' def test_validator_warning_master_slave(): @@ -104,18 +118,38 @@ def test_validator_warning_master_slave(): assert interface1.impl_get_group_type() == groups.master root = OptionDescription('root', '', [interface1]) cfg = Config(root) - cfg.ip_admin_eth0.ip_admin_eth0.append(None) - assert cfg.cfgimpl_get_values().has_warning() is False - cfg.ip_admin_eth0.netmask_admin_eth0 = ['val1'] - assert cfg.ip_admin_eth0.netmask_admin_eth0 == ['val1'] - assert cfg.cfgimpl_get_values().has_warning() is True - assert cfg.cfgimpl_get_values().get_warnings() == {netmask_admin_eth0: 'invalid value val1 for option netmask_admin_eth0: error'} - cfg.ip_admin_eth0.ip_admin_eth0 = ['val'] - assert cfg.ip_admin_eth0.ip_admin_eth0 == ['val'] - assert cfg.cfgimpl_get_values().get_warnings() == {ip_admin_eth0: 'invalid value val for option ip_admin_eth0: error'} - cfg.ip_admin_eth0.ip_admin_eth0 = ['val', 'val1', 'val1'] - assert cfg.cfgimpl_get_values().get_warnings() == {ip_admin_eth0: 'invalid value val for option ip_admin_eth0: error'} - cfg.ip_admin_eth0.ip_admin_eth0 = ['val1', 'val', 'val1'] - assert cfg.cfgimpl_get_values().get_warnings() == {ip_admin_eth0: 'invalid value val for option ip_admin_eth0: error'} - cfg.ip_admin_eth0.ip_admin_eth0 = ['val1', 'val1', 'val'] - assert cfg.cfgimpl_get_values().get_warnings() == {ip_admin_eth0: 'invalid value val for option ip_admin_eth0: error'} + warnings.simplefilter("always", ValueWarning) + with warnings.catch_warnings(record=True) as w: + cfg.ip_admin_eth0.ip_admin_eth0.append(None) + assert w == [] + # + with warnings.catch_warnings(record=True) as w: + cfg.ip_admin_eth0.netmask_admin_eth0 = ['val1'] + assert len(w) == 1 + assert w[0].message.opt == netmask_admin_eth0 + assert str(w[0].message) == 'invalid value val1 for option netmask_admin_eth0: error' + # + with warnings.catch_warnings(record=True) as w: + cfg.ip_admin_eth0.ip_admin_eth0 = ['val'] + assert len(w) == 1 + assert w[0].message.opt == ip_admin_eth0 + assert str(w[0].message) == 'invalid value val for option ip_admin_eth0: error' + # + with warnings.catch_warnings(record=True) as w: + cfg.ip_admin_eth0.ip_admin_eth0 = ['val', 'val1', 'val1'] + assert len(w) == 1 + assert w[0].message.opt == ip_admin_eth0 + assert str(w[0].message) == 'invalid value val for option ip_admin_eth0: error' + # + with warnings.catch_warnings(record=True) as w: + cfg.ip_admin_eth0.ip_admin_eth0 = ['val1', 'val', 'val1'] + assert len(w) == 1 + assert w[0].message.opt == ip_admin_eth0 + assert str(w[0].message) == 'invalid value val for option ip_admin_eth0: error' + # + warnings.resetwarnings() + with warnings.catch_warnings(record=True) as w: + cfg.ip_admin_eth0.ip_admin_eth0 = ['val1', 'val1', 'val'] + assert len(w) == 1 + assert w[0].message.opt == ip_admin_eth0 + assert str(w[0].message) == 'invalid value val for option ip_admin_eth0: error' diff --git a/tiramisu/error.py b/tiramisu/error.py index 6c92be3..a4b3f41 100644 --- a/tiramisu/error.py +++ b/tiramisu/error.py @@ -61,3 +61,35 @@ class SlaveError(Exception): class ConstError(TypeError): "no uniq value in _NameSpace" pass + + +#Warning +class ValueWarning(UserWarning): + """Option could warn user and not raise ValueError. + + Example: + + >>> import warnings + >>> from tiramisu.error import ValueWarning + >>> from tiramisu.option import StrOption, OptionDescription + >>> from tiramisu.config import Config + >>> warnings.simplefilter("always", ValueWarning) + >>> def a(val): + ... raise ValueError('pouet') + ... + >>> s=StrOption('s', '', validator=a, only_warning=True) + >>> o=OptionDescription('o', '', [s]) + >>> c=Config(o) + >>> c.s = 'val' + StrOption:0: ValueWarning: invalid value val for option s: pouet + >>> with warnings.catch_warnings(record=True) as w: + ... c.s = 'val' + ... + >>> w[0].message.opt == s + True + >>> print str(w[0].message) + invalid value val for option s: pouet + """ + def __init__(self, msg, opt): + self.opt = opt + super(ValueWarning, self).__init__(msg) diff --git a/tiramisu/option.py b/tiramisu/option.py index c49e5e4..74f9899 100644 --- a/tiramisu/option.py +++ b/tiramisu/option.py @@ -25,8 +25,9 @@ import sys from copy import copy, deepcopy from types import FunctionType from IPy import IP +import warnings -from tiramisu.error import ConflictError +from tiramisu.error import ConflictError, ValueWarning from tiramisu.setting import groups, multitypes from tiramisu.i18n import _ from tiramisu.autolib import carry_out_calculation @@ -474,7 +475,6 @@ class Option(BaseOption): def do_validation(_value, _index=None): if _value is None: return - ret_validation = None try: # valid with self._validator val_validator(_value) @@ -486,12 +486,13 @@ class Option(BaseOption): msg = _("invalid value {0} for option {1}: {2}").format( _value, self._name, err) if self._only_warning: - ret_validation = msg + warnings.warn_explicit(ValueWarning(msg, self), + ValueWarning, + self.__class__.__name__, 0) else: raise ValueError(msg) # option validation self._validate(_value) - return ret_validation # generic calculation if context is not None: diff --git a/tiramisu/value.py b/tiramisu/value.py index a180806..6942453 100644 --- a/tiramisu/value.py +++ b/tiramisu/value.py @@ -33,7 +33,7 @@ class Values(object): but the values are physicaly located here, in `Values`, wich is also responsible of a caching utility. """ - __slots__ = ('context', '_warning', '_p_', '__weakref__') + __slots__ = ('context', '_p_', '__weakref__') def __init__(self, context, storage): """ @@ -45,7 +45,6 @@ class Values(object): self.context = weakref.ref(context) # the storage type is dictionary or sqlite3 self._p_ = storage - self._warning = {} def _getdefault(self, opt): """ @@ -107,9 +106,9 @@ class Values(object): path = self._get_opt_path(opt) if self._p_.hasvalue(path): setting = self.context().cfgimpl_get_settings() - self._setwarning(opt.impl_validate(opt.impl_getdefault(), - self.context(), - 'validator' in setting), opt) + opt.impl_validate(opt.impl_getdefault(), + self.context(), + 'validator' in setting) self.context().cfgimpl_reset_cache() if (opt.impl_is_multi() and opt.impl_get_multitype() == multitypes.master): @@ -231,8 +230,7 @@ class Values(object): else: value = self._getvalue(opt, path, validate) if config_error is None and validate: - self._setwarning(opt.impl_validate(value, self.context(), - 'validator' in setting), opt) + opt.impl_validate(value, self.context(), 'validator' in setting) if config_error is None and self._is_default_owner(path) and \ 'force_store_value' in setting[opt]: self.setitem(opt, value, path, is_write=False) @@ -253,9 +251,8 @@ class Values(object): # is_write is, for example, used with "force_store_value" # user didn't change value, so not write # valid opt - self._setwarning(opt.impl_validate(value, self.context(), - 'validator' in self.context( - ).cfgimpl_get_settings()), opt) + opt.impl_validate(value, self.context(), + 'validator' in self.context().cfgimpl_get_settings()) if opt.impl_is_multi() and not isinstance(value, Multi): value = Multi(value, self.context, opt, path, setitem=True) self._setvalue(opt, path, value, force_permissive=force_permissive, @@ -374,26 +371,6 @@ class Values(object): def __setstate__(self, states): self._p_ = states['_p_'] - def _setwarning(self, msg, opt): - if msg is not None: - self._warning[opt] = msg - - def has_warning(self): - """If option is "only_warning", validation error is store in - self._warning. - has_warning just indicate that a warning message is store - """ - return self._warning != {} - - def get_warnings(self): - """Get last warnings messages in self._warning. - We can get only one time those messages. - :returns: {opt: msg, opt1: msg1} - """ - ret = self._warning - self._warning = {} - return ret - # ____________________________________________________________ # multi types @@ -564,9 +541,8 @@ class Multi(list): def _validate(self, value): if value is not None: try: - self.context().cfgimpl_get_values()._setwarning( - self.opt.impl_validate(value, context=self.context(), - force_no_multi=True), self.opt) + self.opt.impl_validate(value, context=self.context(), + force_no_multi=True) except ValueError as err: raise ValueError(_("invalid value {0} " "for option {1}: {2}"