From 605163ab4a003e4ec9400f788f8f79604e826057 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Wed, 11 Apr 2018 16:36:15 +0200 Subject: [PATCH] some improvment --- test/test_config_api.py | 22 +++++++++- test/test_masterslaves.py | 20 +++++++++ test/test_metaconfig.py | 1 + test/test_requires.py | 2 + test/test_symlink.py | 20 +++++++++ tiramisu/api.py | 20 ++++----- tiramisu/config.py | 23 ++++------- tiramisu/error.py | 4 +- tiramisu/setting.py | 21 +++------- tiramisu/value.py | 87 +++++++++++++-------------------------- 10 files changed, 116 insertions(+), 104 deletions(-) diff --git a/test/test_config_api.py b/test/test_config_api.py index 2f01c3f..014e6ab 100644 --- a/test/test_config_api.py +++ b/test/test_config_api.py @@ -116,8 +116,26 @@ def test_make_dict_with_disabled(): config = Config(descr) api = getapi(config) api.property.read_only() - d = api.option.make_dict() - assert d == {"s1.a": False, "int": 42} + assert api.option.make_dict() == {"s1.a": False, "int": 42} + assert api.forcepermissive.option.make_dict() == {"s1.a": False, "int": 42} + assert api.unrestraint.option.make_dict() == {"int": 42, "s1.a": False, "s1.b": False, "s2.a": False, "s2.b": False} + + +def test_make_dict_with_disabled_withoption(): + descr = OptionDescription("opt", "", [ + OptionDescription("s1", "", [ + BoolOption("a", "", default=False), + BoolOption("b", "", default=False, properties=('disabled',))]), + OptionDescription("s2", "", [ + BoolOption("a", "", default=False), + BoolOption("b", "", default=False)], properties=('disabled',)), + IntOption("int", "", default=42)]) + config = Config(descr) + api = getapi(config) + api.property.read_only() + assert api.option.make_dict(withoption="a") == {"s1.a": False} + assert api.forcepermissive.option.make_dict(withoption="a") == {"s1.a": False} + assert api.unrestraint.option.make_dict(withoption="a") == {"s1.a": False, "s1.b": False, "s2.a": False, "s2.b": False} def test_make_dict_with_disabled_in_callback(): diff --git a/test/test_masterslaves.py b/test/test_masterslaves.py index f406454..19a7023 100644 --- a/test/test_masterslaves.py +++ b/test/test_masterslaves.py @@ -163,6 +163,26 @@ def test_groups_with_master(): assert interface1.impl_get_group_type() == groups.master +def test_groups_is_master(): + ip_admin_eth0 = StrOption('ip_admin_eth0', "ip réseau autorisé", multi=True) + netmask_admin_eth0 = StrOption('netmask_admin_eth0', "masque du sous-réseau", multi=True, default_multi='value') + interface1 = MasterSlaves('ip_admin_eth0', '', [ip_admin_eth0, netmask_admin_eth0]) + var = StrOption('var', "ip réseau autorisé", multi=True) + od2 = OptionDescription('od2', '', [var]) + od1 = OptionDescription('od', '', [interface1, od2]) + api = getapi(Config(od1)) + assert not api.option('od2').option.ismasterslaves() + assert api.option('ip_admin_eth0').option.ismasterslaves() + assert not api.option('od2.var').option.ismaster() + assert not api.option('od2.var').option.isslave() + assert api.option('ip_admin_eth0.ip_admin_eth0').option.ismaster() + assert not api.option('ip_admin_eth0.ip_admin_eth0').option.isslave() + assert not api.option('ip_admin_eth0.netmask_admin_eth0').option.ismaster() + assert api.option('ip_admin_eth0.netmask_admin_eth0').option.isslave() + assert api.option('ip_admin_eth0.netmask_admin_eth0').option.path() == 'ip_admin_eth0.netmask_admin_eth0' + assert api.option('ip_admin_eth0.netmask_admin_eth0').option.defaultmulti() == 'value' + + if TIRAMISU_VERSION != 2: def test_groups_with_master_in_root(): ip_admin_eth0 = StrOption('ip_admin_eth0', "ip réseau autorisé", multi=True) diff --git a/test/test_metaconfig.py b/test/test_metaconfig.py index c3da53e..8ca900d 100644 --- a/test/test_metaconfig.py +++ b/test/test_metaconfig.py @@ -235,6 +235,7 @@ def test_not_meta(): raises(ValueError, "GroupConfig(conf1)") #same name raises(ConflictError, "GroupConfig([conf2, conf4], session_id='conf2')") + raises(ConflictError, "GroupConfig([conf2, conf2], session_id='conf8')") grp = GroupConfig([conf1, conf2]) api = getapi(grp) raises(ConfigError, "api.option('od1.i1').value.get()") diff --git a/test/test_requires.py b/test/test_requires.py index fc69aae..c563e7a 100644 --- a/test/test_requires.py +++ b/test/test_requires.py @@ -126,6 +126,8 @@ def test_requires_same_action(): props = err.proptype submsg = '"disabled" (' + _('the value of "{0}" is {1}').format('activate_service', '"False"') + ')' assert str(err) == str(_('cannot access to {0} "{1}" because has {2} {3}').format('option', 'ip_address_service_web', 'property', submsg)) + #access to cache + assert str(err) == str(_('cannot access to {0} "{1}" because has {2} {3}').format('option', 'ip_address_service_web', 'property', submsg)) assert frozenset(props) == frozenset(['disabled']) diff --git a/test/test_symlink.py b/test/test_symlink.py index 2f83351..cdc62d3 100644 --- a/test/test_symlink.py +++ b/test/test_symlink.py @@ -53,6 +53,26 @@ def test_symlink_del_option(): raises(TypeError, "api.option('c').value.reset()") +def test_symlink_addproperties(): + boolopt = BoolOption('b', '', default=True, properties=('test',)) + linkopt = SymLinkOption("c", boolopt) + descr = OptionDescription('opt', '', [boolopt, linkopt]) + api = getapi(Config(descr)) + api.property.read_write() + raises(TypeError, "api.option('c').property.add('new')") + raises(TypeError, "api.option('c').property.reset()") + + +def test_symlink_addpermissive(): + boolopt = BoolOption('b', '', default=True, properties=('test',)) + linkopt = SymLinkOption("c", boolopt) + descr = OptionDescription('opt', '', [boolopt, linkopt]) + api = getapi(Config(descr)) + api.property.read_write() + raises(TypeError, "api.option('c').permissive.set(frozenset(['new']))") + raises(TypeError, "api.option('c').permissive.reset()") + + def test_symlink_getproperties(): boolopt = BoolOption('b', '', default=True, properties=('test',)) linkopt = SymLinkOption("c", boolopt) diff --git a/tiramisu/api.py b/tiramisu/api.py index 932aae4..252ba17 100644 --- a/tiramisu/api.py +++ b/tiramisu/api.py @@ -43,7 +43,7 @@ def count(func): global MOD_COUNT_TIME class_name = func.__str__().split()[1].split('.')[0] func_name = func.__name__ - def wrapper(*args, **kwargs): + def wrapper(*args, **kwargs): # pragma: no cover time1 = time() ret = func(*args, **kwargs) time2 = time() @@ -55,7 +55,7 @@ def count(func): #print('%s function took %0.3f ms' % (func_name, diff)) #print(COUNT_TIME) return ret - if COUNT_TIME is not False: + if COUNT_TIME is not False: # pragma: no cover COUNT_TIME.setdefault(class_name, {}) COUNT_TIME[class_name][func_name] = {'max': 0, 'min': 1000, @@ -67,7 +67,7 @@ def count(func): def display_count(): - if COUNT_TIME is not False: + if COUNT_TIME is not False: # pragma: no cover global MOD_COUNT_TIME #print(MOD_COUNT_TIME) print() @@ -104,7 +104,7 @@ class TiramisuHelp: module = self.registers[module_name] instance_module = module(None) if isinstance(instance_module, TiramisuDispatcher): - if _valid and not getdoc(module.__call__): + if _valid and not getdoc(module.__call__): # pragma: no cover raise Exception('unknown doc for {}'.format('__call__')) module_doc = _(getdoc(module.__call__)) module_signature = signature(module.__call__) @@ -117,12 +117,12 @@ class TiramisuHelp: else: root = root + '[config(path).]' if isinstance(instance_module, CommonTiramisuOption): - if _valid and not getdoc(module): + if _valid and not getdoc(module): # pragma: no cover raise Exception('unknown doc for {}'.format(module.__class__.__name__)) module_doc = _(getdoc(module)) options.append(self.tmpl_help.format(space, self.icon, root + module_name, module_doc)) if isinstance(instance_module, TiramisuContext): - if _valid and not getdoc(module): + if _valid and not getdoc(module): # pragma: no cover raise Exception('unknown doc for {}'.format(module.__class__.__name__)) module_doc = _(getdoc(module)) options.append(self.tmpl_help.format(space, self.icon, root + module_name, module_doc)) @@ -140,11 +140,11 @@ class TiramisuHelp: module_args = '(' + ', '.join(module_args) + ')' if func_name.startswith('_'): func_name = func_name[1:] - if _valid and not getdoc(func): + if _valid and not getdoc(func): # pragma: no cover raise Exception('unknown doc for {}'.format(func.__name__)) options.append(self.tmpl_help.format(space, self.icon, root + func_name + module_args, _(getdoc(func)))) if init: - if _display: + if _display: # pragma: no cover print('\n'.join(options)) else: return options @@ -487,9 +487,9 @@ class TiramisuOptionPermissive(CommonTiramisuOption): permissives=permissives) @count - def reset(self, path): + def reset(self): """reset all personalised permissive""" - self.set(tuple()) + self.set(frozenset()) class TiramisuOptionInformation(CommonTiramisuOption): diff --git a/tiramisu/config.py b/tiramisu/config.py index e92c9cb..103b00e 100644 --- a/tiramisu/config.py +++ b/tiramisu/config.py @@ -493,6 +493,7 @@ class SubConfig(object): "option")) context = self.cfgimpl_get_context() if withoption is not None: + mypath = self.cfgimpl_get_path() for path in context.find(bytype=None, byname=withoption, byvalue=withvalue, @@ -503,7 +504,6 @@ class SubConfig(object): config_bag) sconfig_bag = config_bag.copy('nooption') sconfig_bag.option = opt - mypath = self.cfgimpl_get_path() if mypath is not None: if mypath == path: withoption = None @@ -511,9 +511,9 @@ class SubConfig(object): break else: tmypath = mypath + '.' - if not path.startswith(tmypath): - raise AttributeError(_('unexpected path {0}, ' - 'should start with {1}' + if not path.startswith(tmypath): # pragma: no cover + raise AttributeError(_('unexpected path "{0}", ' + 'should start with "{1}"' '').format(path, mypath)) path = path[len(tmypath):] self._make_sub_dict(path, @@ -578,12 +578,6 @@ class SubConfig(object): if flatten: name = option.impl_getname() elif fullpath: - #FIXME - #root_path = self.cfgimpl_get_path() - #if root_path is None: - # name = opt.impl_getname() - #else: - # name = '.'.join([root_path, opt.impl_getname()]) name = self._get_subpath(name) else: name = '.'.join(_currpath + [name]) @@ -602,8 +596,7 @@ class _CommonConfig(SubConfig): "abstract base class for the Config, GroupConfig and the MetaConfig" __slots__ = ('_impl_values', '_impl_settings', - '_impl_meta', - '_impl_test') + '_impl_meta') def _impl_build_all_caches(self): descr = self.cfgimpl_get_description() @@ -699,7 +692,7 @@ class _CommonConfig(SubConfig): # ____________________________________________________________ class Config(_CommonConfig): "main configuration management entry" - __slots__ = ('__weakref__', '_impl_test', '_impl_name') + __slots__ = ('__weakref__', '_impl_name') def __init__(self, descr, @@ -748,7 +741,6 @@ class Config(_CommonConfig): ConfigBag(self), None) #undocumented option used only in test script - self._impl_test = False if _duplicate is False and (force_settings is None or force_values is None): self._impl_build_all_caches() self._impl_name = session_id @@ -784,7 +776,7 @@ class GroupConfig(_CommonConfig): name = names.pop(0) if name in names: raise ConflictError(_('config name must be uniq in ' - 'groupconfig for {0}').format(name)) + 'groupconfig for "{0}"').format(name)) self._impl_children = children properties, permissives, values, session_id = get_storages(self, session_id, @@ -799,7 +791,6 @@ class GroupConfig(_CommonConfig): ConfigBag(self), None) #undocumented option used only in test script - self._impl_test = False self._impl_name = session_id def cfgimpl_get_children(self): diff --git a/tiramisu/error.py b/tiramisu/error.py index 02637d6..b62f29f 100644 --- a/tiramisu/error.py +++ b/tiramisu/error.py @@ -23,9 +23,7 @@ def display_list(lst, separator='and', add_quote=False): separator = _('and') elif separator == 'or': separator = _('or') - if len(lst) == 0: - return '' - elif len(lst) == 1: + if len(lst) == 1: ret = lst[0] if not isinstance(ret, str): ret = str(ret) diff --git a/tiramisu/setting.py b/tiramisu/setting.py index 09e48cc..061fc09 100644 --- a/tiramisu/setting.py +++ b/tiramisu/setting.py @@ -306,7 +306,7 @@ class Settings(object): old `SubConfig`, `Values`, `Multi` or `Settings`) """ context = self.context() - if context is None: # pragma: optional cover + if context is None: # pragma: no cover raise ConfigError(_('the context does not exist anymore')) return context @@ -576,22 +576,15 @@ class Settings(object): if path is not None and config_bag.option.impl_getrequires() is not None: not_allowed_props = properties & getattr(config_bag.option, '_calc_properties', static_set) if not_allowed_props: - if len(not_allowed_props) == 1: - prop_msg = _('property') - calc_msg = _('this property is calculated') - else: - prop_msg = _('properties') - calc_msg = _('those properties are calculated') - raise ValueError(_('cannot set {} {} for option "{}" {}' - '').format(prop_msg, display_list(list(not_allowed_props), add_quote=True), - config_bag.option.impl_get_display_name(), - calc_msg)) + raise ValueError(_('cannot set property {} for option "{}" this property is calculated' + '').format(display_list(list(not_allowed_props), add_quote=True), + config_bag.option.impl_get_display_name())) if config_bag is None: opt = None else: opt = config_bag.option if opt and opt.impl_is_symlinkoption(): - raise TypeError(_("can't assign properties to the SymLinkOption \"{}\"" + raise TypeError(_("can't assign property to the SymLinkOption \"{}\"" "").format(opt.impl_get_display_name())) if 'force_default_on_freeze' in properties and \ 'frozen' not in properties and \ @@ -599,8 +592,6 @@ class Settings(object): raise ConfigError(_('a master ({0}) cannot have ' '"force_default_on_freeze" property without "frozen"' '').format(opt.impl_get_display_name())) - if not isinstance(properties, frozenset): - raise TypeError(_('properties must be a frozenset')) self._p_.setproperties(path, properties) #values too because of slave values could have a PropertiesOptionError has value @@ -658,7 +649,7 @@ class Settings(object): if opt and opt.impl_is_symlinkoption(): raise TypeError(_("can't reset properties to the SymLinkOption \"{}\"" "").format(opt.impl_get_display_name())) - if all_properties and (path or opt): # pragma: optional cover + if all_properties and (path or opt): raise ValueError(_('opt and all_properties must not be set ' 'together in reset')) if all_properties: diff --git a/tiramisu/value.py b/tiramisu/value.py index 966a70c..47dc0ed 100644 --- a/tiramisu/value.py +++ b/tiramisu/value.py @@ -291,21 +291,18 @@ class Values(object): force_allow_empty_list=False, index=None): "convenience method to know if an option is empty" - if value is undefined: - return False - else: - empty = opt._empty - if index in [None, undefined] and opt.impl_is_multi(): - if force_allow_empty_list: - allow_empty_list = True - else: - allow_empty_list = opt.impl_allow_empty_list() - if allow_empty_list is undefined: - allow_empty_list = opt.impl_is_master_slaves('slave') - isempty = value is None or (not allow_empty_list and value == []) or \ - None in value or empty in value + empty = opt._empty + if index in [None, undefined] and opt.impl_is_multi(): + if force_allow_empty_list: + allow_empty_list = True else: - isempty = value is None or value == empty + allow_empty_list = opt.impl_allow_empty_list() + if allow_empty_list is undefined: + allow_empty_list = opt.impl_is_master_slaves('slave') + isempty = value is None or (not allow_empty_list and value == []) or \ + None in value or empty in value + else: + isempty = value is None or value == empty return isempty #______________________________________________________________________ @@ -528,9 +525,6 @@ class Values(object): if opt.impl_is_symlinkoption(): raise ConfigError(_("can't set owner for the SymLinkOption \"{}\"" "").format(opt.impl_get_display_name())) - if not isinstance(owner, owners.Owner): - raise ConfigError(_("invalid owner {0}").format(str(owner))) - if owner in forbidden_owners: raise ConfigError(_('set owner "{0}" is forbidden').format(str(owner))) @@ -653,11 +647,6 @@ class Values(object): settings = context.cfgimpl_get_settings() # First validate properties with this value self_properties = config_bag.properties - if self_properties is None: - self_properties = settings.getproperties(path, - None, - config_bag) - config_bag.properties = self_properties settings.validate_frozen(path, index, config_bag) @@ -693,10 +682,7 @@ class Values(object): config, od_setting_properties): settings = context.cfgimpl_get_settings() - is_masterslaves = description.impl_is_master_slaves() - lenmaster = None - optmaster = None - pathmaster = None +# for option in config.cfgimpl_get_children(self.config_bag): for option in description.impl_getchildren(config_bag, context): sconfig_bag = config_bag.copy('nooption') sconfig_bag.option = option @@ -704,6 +690,7 @@ class Values(object): path = '.'.join(currpath + [name]) if option.impl_is_optiondescription(): + ori_setting_properties = sconfig_bag.setting_properties sconfig_bag.setting_properties = od_setting_properties try: subconfig = config.getattr(name, @@ -712,8 +699,9 @@ class Values(object): except PropertiesOptionError as err: pass else: + sconfig_bag.setting_properties = ori_setting_properties for path in self._mandatory_warnings(context, - config_bag, + sconfig_bag, option, currpath + [name], subconfig, @@ -729,42 +717,26 @@ class Values(object): sconfig_bag.properties = self_properties if 'mandatory' in self_properties or 'empty' in self_properties: - value = config.getattr(name, - None, - sconfig_bag) - if is_masterslaves: - lenmaster = len(value) - pathmaster = name - optmaster = option + config.getattr(name, + None, + sconfig_bag) else: - if lenmaster is None: - # master is a length (so int) if value is already calculated - # otherwise get value and calculate length - nconfig_bag = config_bag.copy('nooption') - nconfig_bag.option = optmaster - values = config.getattr(pathmaster, - None, - nconfig_bag) - lenmaster = len(values) - for index in range(lenmaster): + for index in range(config.cfgimpl_get_length()): self_properties = settings.getproperties(path, index, sconfig_bag) sconfig_bag.properties = self_properties - values = config.getattr(name, - index, - sconfig_bag) + if 'mandatory' in self_properties: + config.getattr(name, + index, + sconfig_bag) except PropertiesOptionError as err: if err.proptype == ['mandatory']: yield path - if is_masterslaves and lenmaster is None: - break except ConfigError as err: #assume that uncalculated value is an empty value yield path - if is_masterslaves and lenmaster is None: - break def mandatory_warnings(self, config_bag): @@ -783,10 +755,9 @@ class Values(object): config_bag.display_warnings = False descr = context.cfgimpl_get_description() - for path in self._mandatory_warnings(context, - config_bag, - descr, - [], - context, - od_setting_properties): - yield path + return self._mandatory_warnings(context, + config_bag, + descr, + [], + context, + od_setting_properties)