From b2cc5f7913f41d82a44252bc1a1cb237920b3a01 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Fri, 6 Apr 2018 23:51:25 +0200 Subject: [PATCH] remove dead code --- test/test_option.py | 28 ++++- test/test_parsing_group.py | 17 +++ tiramisu/api.py | 16 +-- tiramisu/autolib.py | 7 +- tiramisu/config.py | 153 ++++++++------------------- tiramisu/option/baseoption.py | 2 +- tiramisu/option/option.py | 5 +- tiramisu/option/optiondescription.py | 7 +- tiramisu/setting.py | 3 - 9 files changed, 100 insertions(+), 138 deletions(-) diff --git a/test/test_option.py b/test/test_option.py index 0a86788..0a33e04 100644 --- a/test/test_option.py +++ b/test/test_option.py @@ -6,8 +6,8 @@ do_autopath() from py.test import raises -from tiramisu.option import IntOption, OptionDescription -from tiramisu.config import Config +from tiramisu.error import APIError +from tiramisu import IntOption, OptionDescription, Config, getapi def a_func(): @@ -97,3 +97,27 @@ def test_option_multi(): raises(ValueError, "IntOption('test', '', multi=True, default_multi='yes')") #not default_multi with callback raises(ValueError, "IntOption('test', '', multi=True, default_multi=1, callback=a_func)") + + +def test_unknown_option(): + i = IntOption('test', '') + od1 = OptionDescription('od', '', [i]) + od2 = OptionDescription('od', '', [od1]) + api = getapi(Config(od2)) + # test is an option, not an optiondescription + raises(AttributeError, "api.option('od.test.unknown').value.get()") + # unknown is an unknown option + raises(AttributeError, "api.option('unknown').value.get()") + # unknown is an unknown option + raises(AttributeError, "api.option('od.unknown').value.get()") + # unknown is an unknown optiondescription + raises(AttributeError, "api.option('od.unknown.suboption').value.get()") + + +def test_asign_optiondescription(): + i = IntOption('test', '') + od1 = OptionDescription('od', '', [i]) + od2 = OptionDescription('od', '', [od1]) + api = getapi(Config(od2)) + raises(APIError, "api.option('od').value.set('test')") + raises(APIError, "api.option('od').value.reset()") diff --git a/test/test_parsing_group.py b/test/test_parsing_group.py index 0d6c974..4777587 100644 --- a/test/test_parsing_group.py +++ b/test/test_parsing_group.py @@ -587,3 +587,20 @@ def test_groups_with_master_get_modified_value(): api.option('ip_admin_eth0.ip_admin_eth0').value.set(['192.168.1.1', '192.168.1.1']) api.option('ip_admin_eth0.netmask_admin_eth0', 1).value.set('255.255.255.255') assert api.value.exportation() == (('ip_admin_eth0.ip_admin_eth0', 'ip_admin_eth0.netmask_admin_eth0',), (None, (0, 1)), (('192.168.1.1', '192.168.1.1'), ('255.255.255.255', '255.255.255.255')), ('user', ('user', 'user'))) + + +def test_wrong_index(): + ip_admin_eth0 = StrOption('ip_admin_eth0', "ip réseau autorisé", multi=True, default=['1.1.1.1']) + netmask_admin_eth0 = StrOption('netmask_admin_eth0', "masque du sous-réseau", multi=True) + interface1 = MasterSlaves('ip_admin_eth0', '', [ip_admin_eth0, netmask_admin_eth0]) + od1 = OptionDescription('od', '', [interface1]) + maconfig = OptionDescription('toto', '', [od1]) + api = getapi(Config(maconfig)) + api.property.read_write() + assert api.option('od.ip_admin_eth0.ip_admin_eth0').option.get() + raises(APIError, "api.option('od.ip_admin_eth0.ip_admin_eth0', 0).option.get()") + assert api.option('od.ip_admin_eth0.netmask_admin_eth0', 0).option.get() + assert api.option('od.ip_admin_eth0').option.get() + raises(APIError, "api.option('od.ip_admin_eth0', 0).option.get()") + assert api.option('od').option.get() + raises(APIError, "api.option('od', 0).option.get()") diff --git a/tiramisu/api.py b/tiramisu/api.py index dd98570..4c8f03c 100644 --- a/tiramisu/api.py +++ b/tiramisu/api.py @@ -94,12 +94,14 @@ class CommonTiramisu(object): self.config_bag.config.cfgimpl_get_settings().validate_properties(self.path, self.index, self.config_bag) - if self.index is not None and option.impl_is_master_slaves('slave') and \ - self.index >= self.subconfig.cfgimpl_get_length(): - raise SlaveError(_('index "{}" is higher than the master length "{}" ' - 'for option "{}"').format(self.index, - self.subconfig.cfgimpl_get_length(), - option.impl_get_display_name())) + if self.index is not None: + if option.impl_is_optiondescription() or not option.impl_is_master_slaves('slave'): + raise APIError('index must be set only with a slave option') + if self.index >= self.subconfig.cfgimpl_get_length(): + raise SlaveError(_('index "{}" is higher than the master length "{}" ' + 'for option "{}"').format(self.index, + self.subconfig.cfgimpl_get_length(), + option.impl_get_display_name())) if not self.allow_optiondescription and option.impl_is_optiondescription(): raise APIError(_('option must not be an optiondescription')) return option @@ -841,8 +843,6 @@ class TiramisuDispatcherConfig(TiramisuContextConfig): class TiramisuDispatcherOption(TiramisuContextOption): def __call__(self, path, index=None): - if path is None: - return self config_bag = self.config_bag.copy() validate = not config_bag.force_unrestraint if not validate: diff --git a/tiramisu/autolib.py b/tiramisu/autolib.py index 49088e2..aeadd47 100644 --- a/tiramisu/autolib.py +++ b/tiramisu/autolib.py @@ -183,19 +183,15 @@ def carry_out_calculation(option, if opt == option: index_ = None with_index = False - iter_slave = True elif opt.impl_is_master_slaves('slave'): index_ = index with_index = False - iter_slave = False else: index_ = None with_index = True - iter_slave = False else: index_ = None with_index = False - iter_slave = False if opt == option and orig_value is not undefined and \ (not opt.impl_is_master_slaves('slave') or index is None): value = orig_value @@ -206,8 +202,7 @@ def carry_out_calculation(option, # get value value = context.getattr(path, index_, - sconfig_bag, - iter_slave=iter_slave) + sconfig_bag) if with_index: value = value[index] except PropertiesOptionError as err: diff --git a/tiramisu/config.py b/tiramisu/config.py index e0c1f97..774f63a 100644 --- a/tiramisu/config.py +++ b/tiramisu/config.py @@ -66,9 +66,6 @@ class SubConfig(object): raise TypeError(_('descr must be an optiondescription, not {0}' ).format(type(descr))) self._impl_descr = descr - # sub option descriptions - if not isinstance(context, weakref.ReferenceType): # pragma: optional cover - raise ValueError('context must be a Weakref') self._impl_context = context self._impl_path = subpath if descr is not None and \ @@ -122,7 +119,7 @@ class SubConfig(object): if resetted_opts is None: resetted_opts = [] - context = self._cfgimpl_get_context() + context = self.cfgimpl_get_context() values = context.cfgimpl_get_values() settings = context.cfgimpl_get_settings() @@ -189,7 +186,7 @@ class SubConfig(object): def cfgimpl_get_children(self, config_bag): - context = self._cfgimpl_get_context() + context = self.cfgimpl_get_context() for opt in self.cfgimpl_get_description().impl_getchildren(config_bag): nconfig_bag = config_bag.copy('nooption') nconfig_bag.option = opt @@ -205,23 +202,7 @@ class SubConfig(object): yield name # ______________________________________________________________________ - -# def __str__(self): -# "Config's string representation" -# lines = [] -# for name, grp in self.iter_groups(): -# lines.append("[{0}]".format(name)) -# for name, value in self: -# try: -# lines.append("{0} = {1}".format(name, value)) -# except UnicodeEncodeError: # pragma: optional cover -# lines.append("{0} = {1}".format(name, -# value.encode(default_encoding))) -# return '\n'.join(lines) -# -# __repr__ = __str__ - - def _cfgimpl_get_context(self): + def cfgimpl_get_context(self): """context could be None, we need to test it context is None only if all reference to `Config` object is deleted (for example we delete a `Config` and we manipulate a reference to @@ -232,9 +213,6 @@ class SubConfig(object): raise ConfigError(_('the context does not exist anymore')) return context - def cfgimpl_get_context(self): - return self._cfgimpl_get_context() - def cfgimpl_get_description(self): if self._impl_descr is None: # pragma: optional cover raise ConfigError(_('no option description found for this config' @@ -243,10 +221,10 @@ class SubConfig(object): return self._impl_descr def cfgimpl_get_settings(self): - return self._cfgimpl_get_context()._impl_settings + return self.cfgimpl_get_context()._impl_settings def cfgimpl_get_values(self): - return self._cfgimpl_get_context()._impl_values + return self.cfgimpl_get_context()._impl_values def setattr(self, name, @@ -255,11 +233,7 @@ class SubConfig(object): config_bag, _commit=True): - if name.startswith('_impl_'): - return object.__setattr__(self, - name, - value) - context = self._cfgimpl_get_context() + context = self.cfgimpl_get_context() if '.' in name: # pragma: optional cover # when set_value self, name = self.cfgimpl_get_home_by_path(name, @@ -268,10 +242,7 @@ class SubConfig(object): config_bag.option = self.cfgimpl_get_description().impl_getchild(name, config_bag, self) - if config_bag.option.impl_is_optiondescription() or \ - isinstance(config_bag.option, SynDynOptionDescription): - raise ConfigError(_("can't assign to an OptionDescription")) # pragma: optional cover - elif config_bag.option.impl_is_symlinkoption(): + if config_bag.option.impl_is_symlinkoption(): raise ConfigError(_("can't assign to a SymLinkOption")) else: path = self._get_subpath(name) @@ -295,14 +266,8 @@ class SubConfig(object): if '.' in name: # pragma: optional cover self, name = self.cfgimpl_get_home_by_path(name, config_bag) - if config_bag.option is None: - config_bag.option = self.cfgimpl_get_description().impl_getchild(name, - config_bag, - self) option = config_bag.option - if option.impl_is_optiondescription() or isinstance(option, SynDynOptionDescription): - raise TypeError(_("can't delete an OptionDescription")) # pragma: optional cover - elif option.impl_is_symlinkoption(): + if option.impl_is_symlinkoption(): raise TypeError(_("can't delete a SymLinkOption")) subpath = self._get_subpath(name) values = self.cfgimpl_get_values() @@ -313,17 +278,9 @@ class SubConfig(object): index, config_bag) elif option.impl_is_master_slaves('slave'): - length = self.cfgimpl_get_length() - if index >= length: - raise SlaveError(_('index "{}" is higher than the master length "{}" ' - 'for option "{}"').format(index, - length, - option.impl_get_display_name())) values.reset_slave(subpath, index, config_bag) - else: - raise ValueError(_("can delete value with index only with a master or a slave")) else: values.reset(subpath, config_bag) @@ -338,9 +295,7 @@ class SubConfig(object): def getattr(self, name, index, - config_bag, - returns_option=False, - iter_slave=False): + config_bag): """ attribute notation mechanism for accessing the value of an option :param name: attribute name @@ -351,7 +306,7 @@ class SubConfig(object): self, name = self.cfgimpl_get_home_by_path(name, config_bag) - context = self._cfgimpl_get_context() + context = self.cfgimpl_get_context() option = config_bag.option if option is None: option = self.cfgimpl_get_description().impl_getchild(name, @@ -359,8 +314,6 @@ class SubConfig(object): self) config_bag.option = option if option.impl_is_symlinkoption(): - if returns_option is True: - return option opt = option.impl_getopt() path = context.cfgimpl_get_description().impl_get_path_by_opt(opt) sconfig_bag = config_bag.copy('nooption') @@ -376,33 +329,20 @@ class SubConfig(object): index, config_bag) if option.impl_is_optiondescription(): - if returns_option is True: - return option return SubConfig(option, self._impl_context, config_bag, subpath) if option.impl_is_master_slaves('slave'): - if index is None and not iter_slave: - raise SlaveError(_('index is mandatory for the slave "{}"' - '').format(subpath)) length = self.cfgimpl_get_length() - if index is not None and index >= length: - raise SlaveError(_('index "{}" is higher than the master length "{}" ' - 'for option "{}"').format(index, - length, - option.impl_get_display_name())) slave_len = self.cfgimpl_get_values()._p_.get_max_length(subpath) if slave_len > length: - raise SlaveError(_('slave option "{}" has higher length "{}" than the master length "{}"' - '').format(option.impl_get_display_name(), - slave_len, - length, - subpath)) - elif index: - raise SlaveError(_('index is forbidden for the not slave "{}"' - '').format(subpath)) + raise SlaveError(_('slave option "{}" has higher length "{}" than the master ' + 'length "{}"').format(option.impl_get_display_name(), + slave_len, + length, + subpath)) if option.impl_is_master_slaves('slave') and index is None: value = [] length = self.cfgimpl_get_length() @@ -420,9 +360,6 @@ class SubConfig(object): index, value, config_bag) - #FIXME utiliser le config_bag ! - if returns_option is True: - return option return value def find(self, @@ -439,13 +376,13 @@ class SubConfig(object): :param byvalue: filter by the option's value :returns: list of matching Option objects """ - return self._cfgimpl_get_context()._find(bytype, - byname, - byvalue, - config_bag, - first=False, - type_=type_, - _subpath=self.cfgimpl_get_path(False)) + return self.cfgimpl_get_context()._find(bytype, + byname, + byvalue, + config_bag, + first=False, + type_=type_, + _subpath=self.cfgimpl_get_path(False)) def find_first(self, config_bag, @@ -462,14 +399,14 @@ class SubConfig(object): :param byvalue: filter by the option's value :returns: list of matching Option objects """ - return self._cfgimpl_get_context()._find(bytype, - byname, - byvalue, - config_bag, - first=True, - type_=type_, - _subpath=self.cfgimpl_get_path(False), - raise_if_not_found=raise_if_not_found) + return self.cfgimpl_get_context()._find(bytype, + byname, + byvalue, + config_bag, + first=True, + type_=type_, + _subpath=self.cfgimpl_get_path(False), + raise_if_not_found=raise_if_not_found) def _find(self, bytype, @@ -608,7 +545,7 @@ class SubConfig(object): if withoption is None and withvalue is not undefined: # pragma: optional cover raise ValueError(_("make_dict can't filtering with value without " "option")) - context = self._cfgimpl_get_context() + context = self.cfgimpl_get_context() if withoption is not None: for path in context._find(bytype=None, byname=withoption, @@ -712,7 +649,7 @@ class SubConfig(object): dyn=True): descr = self.cfgimpl_get_description() if not dyn and descr.impl_is_dynoptiondescription(): - context_descr = self._cfgimpl_get_context().cfgimpl_get_description() + context_descr = self.cfgimpl_get_context().cfgimpl_get_description() return context_descr.impl_get_path_by_opt(descr.impl_getopt()) return self._impl_path @@ -724,14 +661,12 @@ class _CommonConfig(SubConfig): '_impl_meta', '_impl_test') - def _impl_build_all_caches(self, - force_store_values): + def _impl_build_all_caches(self): descr = self.cfgimpl_get_description() if not descr.impl_already_build_caches(): descr._build_cache_option() descr._build_cache(self) - descr.impl_build_force_store_values(self, - force_store_values) + descr.impl_build_force_store_values(self) def unwrap_from_path(self, path, @@ -753,9 +688,10 @@ class _CommonConfig(SubConfig): else: if option.impl_is_symlinkoption(): true_option = option.impl_getopt() - true_path = true_option.impl_getpath(self._cfgimpl_get_context()) - self, path = self.cfgimpl_get_context().cfgimpl_get_home_by_path(true_path, - config_bag) + context = self.cfgimpl_get_context() + true_path = true_option.impl_getpath(context) + self, path = context.cfgimpl_get_home_by_path(true_path, + config_bag) config_bag.option = true_option else: true_path = path @@ -827,8 +763,7 @@ class Config(_CommonConfig): persistent=False, force_values=None, force_settings=None, - _duplicate=False, - _force_store_values=True): + _duplicate=False): """ Configuration option management master class :param descr: describes the configuration schema @@ -871,7 +806,7 @@ class Config(_CommonConfig): #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(_force_store_values) + self._impl_build_all_caches() self._impl_name = session_id def impl_getname(self): @@ -901,7 +836,7 @@ class GroupConfig(_CommonConfig): name_ = child._impl_name names.append(name_) if len(names) != len(set(names)): - for idx in xrange(1, len(names) + 1): + for idx in range(1, len(names) + 1): name = names.pop(0) if name in names: raise ConflictError(_('config name must be uniq in ' @@ -1060,16 +995,14 @@ class MetaConfig(GroupConfig): children, session_id=None, persistent=False, - optiondescription=None, - _force_store_values=True): + optiondescription=None): descr = None if optiondescription is not None: new_children = [] for child_session_id in children: new_children.append(Config(optiondescription, persistent=persistent, - session_id=child_session_id, - _force_store_values=_force_store_values)) + session_id=child_session_id)) children = new_children for child in children: if not isinstance(child, _CommonConfig): diff --git a/tiramisu/option/baseoption.py b/tiramisu/option/baseoption.py index faee934..d88d2d1 100644 --- a/tiramisu/option/baseoption.py +++ b/tiramisu/option/baseoption.py @@ -329,7 +329,7 @@ class Base(object): callback_params = self._build_calculator_params(callback, callback_params) val = getattr(self, '_val_call', (None,))[0] - if callback_params is None or callback_params == {}: + if callback_params == {}: val_call = (callback,) else: val_call = tuple([callback, callback_params]) diff --git a/tiramisu/option/option.py b/tiramisu/option/option.py index 7463669..db43e98 100644 --- a/tiramisu/option/option.py +++ b/tiramisu/option/option.py @@ -110,7 +110,7 @@ class Option(OnlyOption): validator_params = self._build_calculator_params(validator, validator_params, add_value=True) - if validator_params is None: + if validator_params == {}: val_call = (validator,) else: val_call = (validator, validator_params) @@ -575,8 +575,7 @@ class Option(OnlyOption): try: opt_value = context.getattr(path, index_, - sconfig_bag, - iter_slave=True) + sconfig_bag) except PropertiesOptionError as err: if debug: # pragma: no cover log.debug('propertyerror in _launch_consistency: {0}'.format(err)) diff --git a/tiramisu/option/optiondescription.py b/tiramisu/option/optiondescription.py index 98af09d..d2aa1ef 100644 --- a/tiramisu/option/optiondescription.py +++ b/tiramisu/option/optiondescription.py @@ -164,8 +164,7 @@ class CacheOptionDescription(BaseOption): return False def impl_build_force_store_values(self, - context, - force_store_values): + context): value_setted = False values = context.cfgimpl_get_values() for subpath, option in self._cache_force_store_values: @@ -176,9 +175,7 @@ class CacheOptionDescription(BaseOption): if option._is_subdyn(): raise ConfigError(_('a dynoption ({0}) cannot have ' 'force_store_value property').format(subpath)) - if force_store_values is False: - raise Exception('ok ca existe ...') - if force_store_values and not values._p_.hasvalue(subpath): + if not values._p_.hasvalue(subpath): config_bag = ConfigBag(config=context, option=option) value = values.getvalue(subpath, None, diff --git a/tiramisu/setting.py b/tiramisu/setting.py index b5a26f3..20ac032 100644 --- a/tiramisu/setting.py +++ b/tiramisu/setting.py @@ -777,9 +777,6 @@ class Settings(object): def setowner(self, owner): ":param owner: sets the default value for owner at the Config level" - if not isinstance(owner, - owners.Owner): # pragma: optional cover - raise TypeError(_("invalid generic owner {0}").format(str(owner))) self._owner = owner def getowner(self):