From ec169a8dc6598641dd28f46f9fa806b7a76381f2 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Tue, 4 Sep 2018 08:36:02 +0200 Subject: [PATCH] optimisations --- test/test_option_consistency.py | 63 ++++++++++++++++++++++++++-- tiramisu/autolib.py | 1 + tiramisu/config.py | 2 +- tiramisu/option/baseoption.py | 6 ++- tiramisu/option/option.py | 61 +++++++++++++++------------ tiramisu/option/optiondescription.py | 3 +- tiramisu/option/symlinkoption.py | 5 ++- tiramisu/setting.py | 7 +++- tiramisu/storage/util.py | 2 +- tiramisu/value.py | 8 +++- 10 files changed, 118 insertions(+), 40 deletions(-) diff --git a/test/test_option_consistency.py b/test/test_option_consistency.py index 1ef99f7..6b103d2 100644 --- a/test/test_option_consistency.py +++ b/test/test_option_consistency.py @@ -83,6 +83,22 @@ def test_consistency_warnings_only_more_option(): assert len(w) == 1 +def test_consistency_warnings_only_option(): + a = IntOption('a', '') + b = IntOption('b', '', warnings_only=True) + od = OptionDescription('od', '', [a, b]) + a.impl_add_consistency('not_equal', b) + api = Config(od) + api.option('a').value.set(1) + warnings.simplefilter("always", ValueWarning) + with warnings.catch_warnings(record=True) as w: + api.option('b').value.set(1) + assert w != [] + api.option('a').value.reset() + api.option('b').value.set(1) + raises(ValueError, "api.option('a').value.set(1)") + + def test_consistency_not_equal(): a = IntOption('a', '') b = IntOption('b', '') @@ -786,6 +802,48 @@ def test_consistency_with_callback(): api.option('c').value.get() +def test_consistency_warnings_only_options(): + a = IPOption('a', '', warnings_only=True) + b = IPOption('b', '') + c = NetworkOption('c', '', default='192.168.1.0') + d = NetmaskOption('d', '', default='255.255.255.0', properties=('disabled',)) + od = OptionDescription('od', '', [a, b, c, d]) + a.impl_add_consistency('not_equal', b) + a.impl_add_consistency('in_network', c, d, transitive=False) + api = Config(od) + api.property.read_write() + api.option('a').value.set('192.168.1.1') + raises(ValueError, "api.option('b').value.set('192.168.1.1')") + api.option('a').value.set('192.168.2.1') + # + api.option('a').value.set('192.168.1.1') + api.property.pop('disabled') + with warnings.catch_warnings(record=True) as w: + api.option('a').value.set('192.168.2.1') + assert len(w) == 1 + + +#def test_consistency_warnings_only_options_callback(): +# a = IPOption('a', '', warnings_only=True) +# b = IPOption('b', '') +# c = NetworkOption('c', '', default='192.168.1.0') +# d = NetmaskOption('d', '', callback=return_netmask2, callback_params=Params(ParamOption(a))) +# od = OptionDescription('od', '', [a, b, c, d]) +# a.impl_add_consistency('not_equal', b) +# a.impl_add_consistency('in_network', c, d, transitive=False) +# api = Config(od) +# api.property.read_write() +# api.option('a').value.set('192.168.1.1') +# raises(ValueError, "api.option('b').value.set('192.168.1.1')") +# api.option('a').value.set('192.168.2.1') +# # +# api.option('a').value.set('192.168.1.1') +# api.property.pop('disabled') +# with warnings.catch_warnings(record=True) as w: +# api.option('a').value.set('192.168.2.1') +# assert len(w) == 1 + + def test_consistency_double_warnings(): a = IntOption('a', '') b = IntOption('b', '', 1) @@ -802,10 +860,7 @@ def test_consistency_double_warnings(): assert len(w) == 2 with warnings.catch_warnings(record=True) as w: api.option('od.c').value.set(2) - if TIRAMISU_VERSION == 2: - assert len(w) == 0 - else: - assert len(w) == 1 + assert len(w) == 0 with warnings.catch_warnings(record=True) as w: api.option('od.a').value.set(2) assert w != [] diff --git a/tiramisu/autolib.py b/tiramisu/autolib.py index 9767f05..2fbba59 100644 --- a/tiramisu/autolib.py +++ b/tiramisu/autolib.py @@ -71,6 +71,7 @@ def manager_callback(callbk: Union[ParamOption, ParamValue], # don't validate if option is option that we tried to validate config_bag = option_bag.config_bag.copy() config_bag.set_permissive() + config_bag.properties -= {'warnings'} soption_bag = OptionBag() soption_bag.set_option(opt, path, diff --git a/tiramisu/config.py b/tiramisu/config.py index be01ac5..3afdbf4 100644 --- a/tiramisu/config.py +++ b/tiramisu/config.py @@ -626,7 +626,7 @@ class SubConfig(object): descr = self.cfgimpl_get_description() if not dyn and descr.impl_is_dynoptiondescription(): context_descr = self.cfgimpl_get_context().cfgimpl_get_description() - return context_descr.impl_get_path_by_opt(descr.impl_getopt()) + return descr.impl_getopt().impl_getpath(context_descr) return self._impl_path diff --git a/tiramisu/option/baseoption.py b/tiramisu/option/baseoption.py index 06bcf57..c696fb1 100644 --- a/tiramisu/option/baseoption.py +++ b/tiramisu/option/baseoption.py @@ -57,6 +57,7 @@ class Base(object): """Base use by all *Option* classes (Option, OptionDescription, SymLinkOption, ...) """ __slots__ = ('_name', + '_path', '_informations', #calcul '_subdyn', @@ -407,7 +408,7 @@ class BaseOption(Base): is_readonly = True elif name != '_readonly': is_readonly = self.impl_is_readonly() - if is_readonly: + if is_readonly and (name != '_path' or value != getattr(self, '_path', value)): raise AttributeError(_('"{}" ({}) object attribute "{}" is' ' read-only').format(self.__class__.__name__, self.impl_get_display_name(), @@ -416,7 +417,8 @@ class BaseOption(Base): def impl_getpath(self, context): - return context.cfgimpl_get_description().impl_get_path_by_opt(self) + return self._path + #return context.cfgimpl_get_description().impl_get_path_by_opt(self) def impl_has_callback(self): "to know if a callback has been defined or not" diff --git a/tiramisu/option/option.py b/tiramisu/option/option.py index 4828ed5..ad042a4 100644 --- a/tiramisu/option/option.py +++ b/tiramisu/option/option.py @@ -194,6 +194,7 @@ class Option(OnlyOption): else: config_bag = undefined force_index = None + is_warnings_only = getattr(self, '_warnings_only', False) if check_error and config_bag is not undefined and \ not 'validator' in config_bag.properties: @@ -201,7 +202,8 @@ class Option(OnlyOption): self.valid_consistency(option_bag, value, context, - check_error) + check_error, + is_warnings_only) return def _is_not_unique(value): @@ -250,15 +252,27 @@ class Option(OnlyOption): option_bag.ori_option) if ((check_error and not is_warnings_only) or (not check_error and is_warnings_only)): - calculation_validator(_value, - _index) - self._second_level_validation(_value, - is_warnings_only) - + try: + calculation_validator(_value, + _index) + self._second_level_validation(_value, + is_warnings_only) + except ValueError as err: + if is_warnings_only: + msg = _('attention, "{0}" could be an invalid {1} for "{2}"' + '').format(val, + self._display_name, + self.impl_get_display_name()) + err_msg = '{0}'.format(err) + if err_msg: + msg += ', {}'.format(err_msg) + warnings.warn_explicit(ValueWarning(msg, weakref.ref(self)), + ValueWarning, + self.__class__.__name__, 0) + else: + raise err #if is_multi is None: # is_multi = self.impl_is_multi() - - is_warnings_only = getattr(self, '_warnings_only', False) try: val = value if not self.impl_is_multi(): @@ -294,33 +308,24 @@ class Option(OnlyOption): self.valid_consistency(option_bag, value, context, - check_error) + check_error, + is_warnings_only) except ValueError as err: + #raise err if debug: # pragma: no cover log.debug('do_validation: value: {0}, index: {1}:' ' {2}'.format(val, force_index, err), exc_info=True) - if is_warnings_only: - msg = _('attention, "{0}" could be an invalid {1} for "{2}"' - '').format(val, - self._display_name, - self.impl_get_display_name()) - else: - msg = _('"{0}" is an invalid {1} for "{2}"' - '').format(val, - self._display_name, - self.impl_get_display_name()) + msg = _('"{0}" is an invalid {1} for "{2}"' + '').format(val, + self._display_name, + self.impl_get_display_name()) err_msg = '{0}'.format(err) if err_msg: msg += ', {}'.format(err_msg) - if check_error: - raise ValueError(msg) - else: - warnings.warn_explicit(ValueWarning(msg, weakref.ref(self)), - ValueWarning, - self.__class__.__name__, 0) + raise ValueError(msg) def impl_is_dynsymlinkoption(self): @@ -449,7 +454,8 @@ class Option(OnlyOption): option_bag, value, context, - check_error): + check_error, + option_warnings_only): if context is not undefined: descr = context.cfgimpl_get_description() # no consistency found at all @@ -468,9 +474,10 @@ class Option(OnlyOption): cconfig_bag = undefined else: cconfig_bag = option_bag.config_bag.copy() + cconfig_bag.properties = cconfig_bag.properties - {'warnings'} cconfig_bag.set_permissive() for cons_id, func, all_cons_opts, params in consistencies: - warnings_only = params.get('warnings_only', False) + warnings_only = option_warnings_only or params.get('warnings_only', False) if (warnings_only and not check_error) or (not warnings_only and check_error): transitive = params.get('transitive', True) #all_cons_opts[0] is the option where func is set diff --git a/tiramisu/option/optiondescription.py b/tiramisu/option/optiondescription.py index 55a5ef1..9d28fb4 100644 --- a/tiramisu/option/optiondescription.py +++ b/tiramisu/option/optiondescription.py @@ -213,6 +213,7 @@ class CacheOptionDescription(BaseOption): path = str('.'.join(_currpath + [attr])) cache_option.append(option) cache_path.append(path) + option._path = path if option.impl_is_optiondescription(): _currpath.append(attr) option._build_cache_option(_currpath, @@ -230,7 +231,7 @@ class OptionDescriptionWalk(CacheOptionDescription): option_bag): option = option_bag.option dynopt = option.getsubdyn() - rootpath = self.impl_get_path_by_opt(dynopt) + rootpath = dynopt.impl_getpath(option_bag.config_bag.context) ori_index = len(rootpath) + 1 subpaths = [rootpath] + option.impl_getpath( option_bag.config_bag.context)[ori_index:].split('.')[:-1] diff --git a/tiramisu/option/symlinkoption.py b/tiramisu/option/symlinkoption.py index b3c31ad..8c13dc9 100644 --- a/tiramisu/option/symlinkoption.py +++ b/tiramisu/option/symlinkoption.py @@ -23,6 +23,7 @@ from ..i18n import _ class SymLinkOption(OnlyOption): + __slots__ = ('_opt',) def __init__(self, name, @@ -39,6 +40,8 @@ class SymLinkOption(OnlyOption): def __getattr__(self, name): + if name == '_path': + return return getattr(self._opt, name) def impl_has_dependency(self, @@ -55,7 +58,7 @@ class SymLinkOption(OnlyOption): return self._opt def impl_is_readonly(self): - return True + return self._path is not None def get_consistencies(self): return () diff --git a/tiramisu/setting.py b/tiramisu/setting.py index 4112419..3ae3b70 100644 --- a/tiramisu/setting.py +++ b/tiramisu/setting.py @@ -138,7 +138,7 @@ class OptionBag: index, config_bag): if path is None: - path = config_bag.context.cfgimpl_get_description().impl_get_path_by_opt(option) + path = option.impl_getpath(config_bag.context) self.path = path self.index = index self.option = option @@ -156,6 +156,11 @@ class OptionBag: return True raise KeyError('unknown key {} for OptionBag'.format(key)) + def __delattr__(self, key): + if key == 'properties': + return + raise KeyError('unknown key {} for ConfigBag'.format(key)) + def copy(self): kwargs = {} option_bag = OptionBag() diff --git a/tiramisu/storage/util.py b/tiramisu/storage/util.py index 9566bca..7a1fa0a 100644 --- a/tiramisu/storage/util.py +++ b/tiramisu/storage/util.py @@ -88,7 +88,7 @@ class Cache(DictCache): print('getcache expired value for path {} < {}'.format( timestamp + expires_time, ntime)) # if expired, remove from cache - self.delcache(path) + #self.delcache(path) else: if DEBUG: print('getcache in cache (2)', path, value, _display_classname(self), diff --git a/tiramisu/value.py b/tiramisu/value.py index b27a84c..77a219e 100644 --- a/tiramisu/value.py +++ b/tiramisu/value.py @@ -84,6 +84,8 @@ class Values(object): # validate value context = self._getcontext() opt = option_bag.option + #print('===', option_bag.path) + #if not is_cached: opt.impl_validate(value, option_bag, context=context, @@ -160,13 +162,15 @@ class Values(object): opt = option_bag.option index = option_bag.index def _reset_cache(_value): + if not 'expire' in option_bag.properties: + return is_cache, cache_value = self._p_.getcache(option_bag.path, - expires_time, + None, index, config_bag.properties, option_bag.properties, 'value') - if is_cache and cache_value == _value: + if not is_cache or cache_value == _value: # calculation return same value as previous value, # so do not invalidate cache return