From 8b3da4d37b2ee7d459dd67adccfab8a0e621ff29 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Thu, 15 Nov 2018 08:15:55 +0100 Subject: [PATCH] simplify tiramisu/option/masterslaves.py --- test/auto/test_auto.py | 2 +- test/test_masterslaves.py | 24 +-- tiramisu/config.py | 7 +- tiramisu/option/masterslaves.py | 169 ++++++++++----------- tiramisu/option/option.py | 7 +- tiramisu/option/optiondescription.py | 4 +- tiramisu/option/syndynoptiondescription.py | 2 +- 7 files changed, 101 insertions(+), 114 deletions(-) diff --git a/test/auto/test_auto.py b/test/auto/test_auto.py index 7e8da5e..fd3e60a 100644 --- a/test/auto/test_auto.py +++ b/test/auto/test_auto.py @@ -1077,7 +1077,7 @@ def autocheck_find(cfg, mcfg, pathread, pathwrite, confread, confwrite, **kwargs def _getoption(opt): opt = opt.option.get() if opt.impl_is_dynsymlinkoption(): - opt = opt._opt + opt = opt.opt return opt def _getoptions(opts): diff --git a/test/test_masterslaves.py b/test/test_masterslaves.py index 1267843..e1ae87a 100644 --- a/test/test_masterslaves.py +++ b/test/test_masterslaves.py @@ -217,25 +217,25 @@ def test_groups_with_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]) + interface1 = MasterSlaves('masterslaves', '', [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 = Config(od1) assert not api.option('od2').option.ismasterslaves() - assert api.option('ip_admin_eth0').option.ismasterslaves() + assert api.option('masterslaves').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.ismulti() - assert api.option('ip_admin_eth0.netmask_admin_eth0').option.ismulti() - assert not api.option('ip_admin_eth0.ip_admin_eth0').option.issubmulti() - assert not api.option('ip_admin_eth0.netmask_admin_eth0').option.issubmulti() - 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' + assert api.option('masterslaves.ip_admin_eth0').option.ismulti() + assert api.option('masterslaves.netmask_admin_eth0').option.ismulti() + assert not api.option('masterslaves.ip_admin_eth0').option.issubmulti() + assert not api.option('masterslaves.netmask_admin_eth0').option.issubmulti() + assert api.option('masterslaves.ip_admin_eth0').option.ismaster() + assert not api.option('masterslaves.ip_admin_eth0').option.isslave() + assert not api.option('masterslaves.netmask_admin_eth0').option.ismaster() + assert api.option('masterslaves.netmask_admin_eth0').option.isslave() + assert api.option('masterslaves.netmask_admin_eth0').option.path() == 'masterslaves.netmask_admin_eth0' + assert api.option('masterslaves.netmask_admin_eth0').option.defaultmulti() == 'value' if TIRAMISU_VERSION != 2: diff --git a/tiramisu/config.py b/tiramisu/config.py index 89b0419..c8f89c5 100644 --- a/tiramisu/config.py +++ b/tiramisu/config.py @@ -239,10 +239,9 @@ class SubConfig(object): raise ConfigError(_("can't assign to a SymLinkOption")) context = option_bag.config_bag.context context.cfgimpl_get_settings().validate_properties(option_bag) - if option_bag.option.impl_is_master_slaves('master'): - self.cfgimpl_get_description().impl_validate_value(option_bag.option, - value, - self) + if option_bag.option.impl_is_master_slaves('master') and len(value) < self._impl_length: + raise SlaveError(_('cannot reduce length of the master "{}"' + '').format(option_bag.option.impl_get_display_name())) return context.cfgimpl_get_values().setvalue(value, option_bag, _commit) diff --git a/tiramisu/option/masterslaves.py b/tiramisu/option/masterslaves.py index 347725c..d9d449b 100644 --- a/tiramisu/option/masterslaves.py +++ b/tiramisu/option/masterslaves.py @@ -21,26 +21,30 @@ # ____________________________________________________________ import weakref from itertools import chain +from typing import List, Iterator, Optional, Any from ..i18n import _ -from ..setting import groups, undefined, OptionBag +from ..setting import groups, undefined, OptionBag, Settings +from ..value import Values from .optiondescription import OptionDescription from .syndynoptiondescription import SynDynMasterSlaves +from .baseoption import BaseOption from .option import Option from ..error import SlaveError, PropertiesOptionError, RequirementError from ..function import ParamOption class MasterSlaves(OptionDescription): - __slots__ = ('master', 'slaves') + __slots__ = ('master', + 'slaves') def __init__(self, - name, - doc, - children, + name: str, + doc: str, + children: List[BaseOption], requires=None, - properties=None): + properties=None) -> None: super(MasterSlaves, self).__init__(name, doc, @@ -50,26 +54,29 @@ class MasterSlaves(OptionDescription): self._group_type = groups.master slaves = [] if len(children) < 2: - raise ValueError(_('a master and a slave are mandatories in masterslaves "{}"').format(name)) + raise ValueError(_('a master and a slave are mandatories in masterslaves "{}"' + '').format(name)) master = children[0] for idx, child in enumerate(children): - if child.impl_is_symlinkoption(): - raise ValueError(_('masterslaves "{0}" shall not have ' - "a symlinkoption").format(self.impl_get_display_name())) - if not isinstance(child, Option): - raise ValueError(_('masterslaves "{0}" shall not have ' - 'a subgroup').format(self.impl_get_display_name())) - if not child.impl_is_multi(): - raise ValueError(_('only multi option allowed in masterslaves "{0}" but option ' - '"{1}" is not a multi').format(self.impl_get_display_name(), - child.impl_get_display_name())) - if idx != 0 and child.impl_getdefault() != []: - raise ValueError(_('not allowed default value for option "{0}" ' - 'in masterslaves "{1}"' - '').format(child.impl_get_display_name(), - self.impl_get_display_name())) - # no empty property for save + if __debug__: + if child.impl_is_symlinkoption(): + raise ValueError(_('masterslaves "{0}" shall not have ' + "a symlinkoption").format(self.impl_get_display_name())) + if not isinstance(child, Option): + raise ValueError(_('masterslaves "{0}" shall not have ' + 'a subgroup').format(self.impl_get_display_name())) + if not child.impl_is_multi(): + raise ValueError(_('only multi option allowed in masterslaves "{0}" but option ' + '"{1}" is not a multi' + '').format(self.impl_get_display_name(), + child.impl_get_display_name())) + if idx != 0 and child.impl_getdefault() != []: + raise ValueError(_('not allowed default value for option "{0}" ' + 'in masterslaves "{1}"' + '').format(child.impl_get_display_name(), + self.impl_get_display_name())) if idx != 0: + # remove empty property for slave child_properties = list(child._properties) child_properties.remove('empty') child._properties = frozenset(child_properties) @@ -77,63 +84,59 @@ class MasterSlaves(OptionDescription): child._add_dependency(self) child._master_slaves = weakref.ref(self) callback, callback_params = master.impl_get_callback() - if callback is not None and callback_params != None: + if callback is not None and callback_params is not None: for callbk in chain(callback_params.args, callback_params.kwargs.values()): - if isinstance(callbk, ParamOption): - if callbk.option in slaves: - raise ValueError(_("callback of master's option shall " - "not refered a slave's ones")) + if isinstance(callbk, ParamOption) and callbk.option in slaves: + raise ValueError(_("callback of master's option shall " + "not refered to a slave's ones")) # master should not have requires, only MasterSlaves should have # so move requires to MasterSlaves - # if MasterSlaves has requires to, cannot manage the move so raises + # if MasterSlaves has requires too, cannot manage this move so raises master_requires = getattr(master, '_requires', None) if master_requires: - if self.impl_getrequires(): + if __debug__ and self.impl_getrequires(): raise RequirementError(_('master {} have requirement, but MasterSlaves {} too' '').format(master.impl_getname(), self.impl_getname())) master_calproperties = getattr(master, '_calc_properties', None) if master_calproperties: - if properties is not None: + if __debug__ and properties is not None: self.validate_properties(name, master_calproperties, frozenset(properties)) setattr(self, '_calc_properties', master_calproperties) setattr(self, '_requires', master_requires) delattr(master, '_requires') - all_requires = getattr(self, '_requires', None) - if all_requires: - for requires_ in all_requires: + if __debug__: + for requires_ in getattr(self, '_requires', ()): for require in requires_: for require_opt, values in require[0]: - if require_opt.impl_is_multi(): - if require_opt.impl_is_master_slaves(): - raise ValueError(_('malformed requirements option "{0}" ' - 'must not be in slave for "{1}"').format( - require_opt.impl_getname(), self.impl_getname())) + if require_opt.impl_is_multi() and require_opt.impl_is_master_slaves(): + raise ValueError(_('malformed requirements option "{0}" ' + 'must not be in slave for "{1}"').format( + require_opt.impl_getname(), + self.impl_getname())) - def is_master(self, opt): - master = self._children[0][0] - return opt.impl_getname() == master or (opt.impl_is_dynsymlinkoption() and - opt.opt.impl_getname() == master) + def is_master(self, + opt: Option) -> bool: + master = self.getmaster() + return opt == master or (opt.impl_is_dynsymlinkoption() and opt.opt == master) - def getmaster(self): + def getmaster(self) -> Option: return self._children[1][0] - def getslaves(self): + def getslaves(self) -> Iterator[Option]: for slave in self._children[1][1:]: yield slave - def in_same_group(self, opt): + def in_same_group(self, + opt: Option) -> bool: if opt.impl_is_dynsymlinkoption(): - c_opt = opt.opt - else: - c_opt = opt - return c_opt in self._children[1] + opt = opt.opt + return opt in self._children[1] def reset(self, - values, - option_bag, - _commit=True): - + values: Values, + option_bag: OptionBag, + _commit: bool=True) -> None: config_bag = option_bag.config_bag.copy() config_bag.remove_validation() for slave in self.getslaves(): @@ -147,12 +150,12 @@ class MasterSlaves(OptionDescription): _commit=_commit) def pop(self, - values, - index, - option_bag, - slaves=undefined): - + values: Values, + index: int, + option_bag: OptionBag, + slaves: Optional[List[Option]]=undefined) -> None: if slaves is undefined: + # slaves is not undefined only in SynDynMasterSlaves slaves = self.getslaves() config_bag = option_bag.config_bag.copy() config_bag.remove_validation() @@ -167,11 +170,10 @@ class MasterSlaves(OptionDescription): # do not check force_default_on_freeze soption_bag.properties = set() if not values.is_default_owner(soption_bag, - validate_meta=False): - if slavelen > index: - values._p_.resetvalue_index(slave_path, - index, - True) + validate_meta=False) and slavelen > index: + values._p_.resetvalue_index(slave_path, + index, + True) if slavelen > index + 1: for idx in range(index + 1, slavelen): if values._p_.hasvalue(slave_path, idx): @@ -179,32 +181,29 @@ class MasterSlaves(OptionDescription): idx) def reset_cache(self, - path, - values, - settings, - resetted_opts): - master = self.getmaster() - slaves = self.getslaves() + path: str, + values: Values, + settings: Settings, + resetted_opts: List[Option]) -> None: self._reset_cache(path, - master, - slaves, + self.getmaster(), + self.getslaves(), values, settings, resetted_opts) def _reset_cache(self, - path, - master, - slaves, - values, - settings, - resetted_opts): + path: str, + master: Option, + slaves: List[Option], + values: Values, + settings: Settings, + resetted_opts: List[Option]) -> None: super().reset_cache(path, values, settings, resetted_opts) - mpath = master.impl_getpath() - master.reset_cache(mpath, + master.reset_cache(master.impl_getpath(), values, settings, None) @@ -216,15 +215,7 @@ class MasterSlaves(OptionDescription): None) resetted_opts.append(spath) - def impl_validate_value(self, - option, - value, - context): - if isinstance(value, list) and len(value) < context._impl_length: - raise SlaveError(_('cannot reduce length of the master "{}"' - '').format(option.impl_get_display_name())) - - def impl_is_master_slaves(self, *args, **kwargs): + def impl_is_master_slaves(self) -> None: return True def to_dynoption(self, diff --git a/tiramisu/option/option.py b/tiramisu/option/option.py index 6e8bb04..5266c2a 100644 --- a/tiramisu/option/option.py +++ b/tiramisu/option/option.py @@ -380,11 +380,10 @@ class Option(BaseOption): def impl_is_master_slaves(self, type_='both'): master_slaves = self.impl_get_master_slaves() if master_slaves is not None: - if type_ in ('both', 'master') and \ - master_slaves.is_master(self): + if type_ == 'both': return True - if type_ in ('both', 'slave') and \ - not master_slaves.is_master(self): + is_master = master_slaves.is_master(self) + if (type_ == 'master' and is_master) or (type_ == 'slave' and not is_master): return True return False diff --git a/tiramisu/option/optiondescription.py b/tiramisu/option/optiondescription.py index adcebe6..244626d 100644 --- a/tiramisu/option/optiondescription.py +++ b/tiramisu/option/optiondescription.py @@ -312,9 +312,7 @@ class OptionDescription(OptionDescriptionWalk): def impl_is_dynoptiondescription(self) -> bool: return False - def impl_is_master_slaves(self, - *args, - **kwargs) -> bool: + def impl_is_master_slaves(self) -> bool: return False # ____________________________________________________________ diff --git a/tiramisu/option/syndynoptiondescription.py b/tiramisu/option/syndynoptiondescription.py index 0e6aaf8..096f62a 100644 --- a/tiramisu/option/syndynoptiondescription.py +++ b/tiramisu/option/syndynoptiondescription.py @@ -28,7 +28,7 @@ from .baseoption import BaseOption from .syndynoption import SynDynOption -class SynDynOptionDescription(object): +class SynDynOptionDescription: __slots__ = ('_opt', '_subpath', '_suffix')