From d75cef9c0fb6980c9a01f17fe616519d2e1cac7f Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Tue, 27 Aug 2013 17:13:20 +0200 Subject: [PATCH 1/6] pep8 --- tiramisu/setting.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tiramisu/setting.py b/tiramisu/setting.py index e6bc645..d99a02e 100644 --- a/tiramisu/setting.py +++ b/tiramisu/setting.py @@ -222,9 +222,8 @@ def get_storage(context, session_id, persistent): if session_id is None: session_id = gen_id(context) - a=__import__(storage_type.get_storage(), globals(), locals(), + return __import__(storage_type.get_storage(), globals(), locals(), ['Storage'], -1).Storage(session_id, persistent) - return a def list_sessions(): @@ -496,7 +495,7 @@ class Settings(object): "'{1}'").format(path, reqpath)) try: value = self.context()._getattr(reqpath, - force_permissive=True) + force_permissive=True) except PropertiesOptionError, err: if not transitive: continue From 3be005e82e1f81699fe2f3d8854ce9e42f18d80f Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Tue, 27 Aug 2013 21:36:52 +0200 Subject: [PATCH 2/6] add test test/test_dereference.py + memoryleaks in optiondescription's cache --- test/test_dereference.py | 111 +++++++++++++++++++++++++++ test/test_storage.py | 3 + tiramisu/option.py | 10 +-- tiramisu/setting.py | 2 +- tiramisu/storage/dictionary/value.py | 2 +- tiramisu/storage/sqlite3/value.py | 2 +- tiramisu/value.py | 2 +- 7 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 test/test_dereference.py diff --git a/test/test_dereference.py b/test/test_dereference.py new file mode 100644 index 0000000..be8dfde --- /dev/null +++ b/test/test_dereference.py @@ -0,0 +1,111 @@ +# coding: utf-8 +import autopath +#from py.test import raises + +from tiramisu.config import Config +from tiramisu.option import BoolOption, OptionDescription +import weakref + + +def test_deref_storage(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + c = Config(o) + w = weakref.ref(c.cfgimpl_get_values()._p_) + del(c) + assert w() is None + + +def test_deref_value(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + c = Config(o) + w = weakref.ref(c.cfgimpl_get_values()) + del(c) + assert w() is None + + +def test_deref_setting(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + c = Config(o) + w = weakref.ref(c.cfgimpl_get_settings()) + del(c) + assert w() is None + + +def test_deref_config(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + c = Config(o) + w = weakref.ref(c) + del(c) + assert w() is None + + +def test_deref_option(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + w = weakref.ref(b) + del(b) + assert w() is not None + del(o) + assert w() is None + + +def test_deref_optiondescription(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + w = weakref.ref(o) + del(b) + assert w() is not None + del(o) + assert w() is None + + +def test_deref_option_cache(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + o.impl_build_cache() + w = weakref.ref(b) + del(b) + assert w() is not None + del(o) + assert w() is None + + +def test_deref_optiondescription_cache(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + o.impl_build_cache() + w = weakref.ref(o) + del(b) + assert w() is not None + del(o) + assert w() is None + + +def test_deref_option_config(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + c = Config(o) + w = weakref.ref(b) + del(b) + assert w() is not None + del(o) + assert w() is not None + del(c) + assert w() is None + + +def test_deref_optiondescription_config(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + c = Config(o) + w = weakref.ref(o) + del(b) + assert w() is not None + del(o) + assert w() is not None + del(c) + assert w() is None diff --git a/test/test_storage.py b/test/test_storage.py index 3857fb5..9b9c430 100644 --- a/test/test_storage.py +++ b/test/test_storage.py @@ -1,3 +1,4 @@ +# coding: utf-8 import autopath #from py.test import raises @@ -79,3 +80,5 @@ def test_create_persistent_retrieve(): delete_session('test_persistent') c = Config(o, session_id='test_persistent', persistent=True) assert c.b is None + +#recup d'un coté de et l'autre diff --git a/tiramisu/option.py b/tiramisu/option.py index 498ddf3..8f598b9 100644 --- a/tiramisu/option.py +++ b/tiramisu/option.py @@ -99,7 +99,7 @@ class Option(BaseInformation): __slots__ = ('_name', '_requires', '_multi', '_validator', '_default_multi', '_default', '_properties', '_callback', '_multitype', '_master_slaves', '_consistencies', '_empty', - '_calc_properties') + '_calc_properties', '__weakref__') _empty = '' def __init__(self, name, doc, default=None, default_multi=None, @@ -714,7 +714,7 @@ class OptionDescription(BaseInformation): """ __slots__ = ('_name', '_requires', '_cache_paths', '_group_type', '_properties', '_children', '_consistencies', - '_calc_properties') + '_calc_properties', '__weakref__') def __init__(self, name, doc, children, requires=None, properties=None): """ @@ -801,8 +801,8 @@ class OptionDescription(BaseInformation): else: save = False if cache_path is None: - cache_path = [self._name] - cache_option = [self] + cache_path = [] + cache_option = [] for option in self.impl_getchildren(): attr = option._name if attr.startswith('_cfgimpl'): @@ -929,7 +929,7 @@ def validate_requires_arg(requires, name): :param requires: have a look at the :meth:`tiramisu.setting.Settings.apply_requires` method to know more about - the description of the requires dictionnary + the description of the requires dictionary """ if requires is None: return None, None diff --git a/tiramisu/setting.py b/tiramisu/setting.py index d99a02e..86f900e 100644 --- a/tiramisu/setting.py +++ b/tiramisu/setting.py @@ -239,7 +239,7 @@ def delete_session(session_id): #____________________________________________________________ class Settings(object): "``Config()``'s configuration options" - __slots__ = ('context', '_owner', '_p_') + __slots__ = ('context', '_owner', '_p_', '__weakref__') def __init__(self, context, storage): """ diff --git a/tiramisu/storage/dictionary/value.py b/tiramisu/storage/dictionary/value.py index e3d41ac..9c3e1f9 100644 --- a/tiramisu/storage/dictionary/value.py +++ b/tiramisu/storage/dictionary/value.py @@ -22,7 +22,7 @@ from tiramisu.storage.dictionary.storage import Cache class Values(Cache): - __slots__ = ('_values',) + __slots__ = ('_values', '__weakref__') def __init__(self, storage): """init plugin means create values storage diff --git a/tiramisu/storage/sqlite3/value.py b/tiramisu/storage/sqlite3/value.py index a70b675..4207c43 100644 --- a/tiramisu/storage/sqlite3/value.py +++ b/tiramisu/storage/sqlite3/value.py @@ -23,7 +23,7 @@ from tiramisu.setting import owners class Values(Cache): - __slots__ = tuple() + __slots__ = ('__weakref__',) def __init__(self, storage): """init plugin means create values storage diff --git a/tiramisu/value.py b/tiramisu/value.py index c654206..1d1babc 100644 --- a/tiramisu/value.py +++ b/tiramisu/value.py @@ -32,7 +32,7 @@ class Values(object): but the values are physicaly located here, in `Values`, wich is also responsible of a caching utility. """ - __slots__ = ('context', '_p_') + __slots__ = ('context', '_p_', '__weakref__') def __init__(self, context, storage): """ From 71a8171c5227ec6ade4f47ec955511e051d135dc Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Tue, 27 Aug 2013 22:08:35 +0200 Subject: [PATCH 3/6] add tests for persistent mode --- test/test_storage.py | 48 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/test/test_storage.py b/test/test_storage.py index 9b9c430..6dbb721 100644 --- a/test/test_storage.py +++ b/test/test_storage.py @@ -4,6 +4,8 @@ import autopath from tiramisu.config import Config from tiramisu.option import BoolOption, OptionDescription +from tiramisu.setting import owners +from tiramisu.setting import list_sessions, delete_session def test_non_persistent(): @@ -54,7 +56,6 @@ def test_delete_session_persistent(): # storage is not persistent pass else: - from tiramisu.setting import list_sessions, delete_session assert 'test_persistent' in list_sessions() delete_session('test_persistent') assert 'test_persistent' not in list_sessions() @@ -75,10 +76,51 @@ def test_create_persistent_retrieve(): del(c) c = Config(o, session_id='test_persistent', persistent=True) assert c.b is True - from tiramisu.setting import list_sessions, delete_session assert 'test_persistent' in list_sessions() delete_session('test_persistent') c = Config(o, session_id='test_persistent', persistent=True) assert c.b is None + delete_session('test_persistent') -#recup d'un coté de et l'autre + +def test_two_persistent(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + try: + c = Config(o, session_id='test_persistent', persistent=True) + except ValueError: + # storage is not persistent + pass + else: + c2 = Config(o, session_id='test_persistent', persistent=True) + assert c.b is None + assert c2.b is None + c.b = False + assert c.b is False + assert c2.b is False + c.b = True + assert c.b is True + assert c2.b is True + delete_session('test_persistent') + + +def test_two_persistent_owner(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + try: + c = Config(o, session_id='test_persistent', persistent=True) + except ValueError: + # storage is not persistent + pass + else: + c2 = Config(o, session_id='test_persistent', persistent=True) + owners.addowner('persistent') + assert c.getowner(b) == owners.default + assert c2.getowner(b) == owners.default + c.b = False + assert c.getowner(b) == owners.user + assert c2.getowner(b) == owners.user + c.cfgimpl_get_values().setowner(b, owners.persistent) + assert c.getowner(b) == owners.persistent + assert c2.getowner(b) == owners.persistent + delete_session('test_persistent') From cb7e4b8893b168439f9a3bf4877d468aa217b0c2 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Tue, 27 Aug 2013 22:21:40 +0200 Subject: [PATCH 4/6] witch => which --- doc/config.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/config.txt b/doc/config.txt index 17bddf3..786665b 100644 --- a/doc/config.txt +++ b/doc/config.txt @@ -6,7 +6,7 @@ Options handling basics Tiramisu is made of almost three main objects : -- :class:`tiramisu.config.Config` witch is the whole configuration entry point +- :class:`tiramisu.config.Config` which is the whole configuration entry point - :class:`tiramisu.option.Option` stands for the option types - :class:`tiramisu.option.OptionDescription` is the shema, the option's structure From 397a600be73828e411e966f83f98a94b507a0670 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Wed, 28 Aug 2013 09:16:12 +0200 Subject: [PATCH 5/6] pep8 --- tiramisu/option.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tiramisu/option.py b/tiramisu/option.py index 8f598b9..31f8cc8 100644 --- a/tiramisu/option.py +++ b/tiramisu/option.py @@ -69,7 +69,7 @@ class BaseInformation(object): except AttributeError: raise AttributeError(_('{0} has no attribute ' 'impl_set_information').format( - self.__class__.__name__)) + self.__class__.__name__)) def impl_get_information(self, key, default=None): """retrieves one information's item @@ -87,7 +87,7 @@ class BaseInformation(object): except AttributeError: raise AttributeError(_('{0} has no attribute ' 'impl_get_information').format( - self.__class__.__name__)) + self.__class__.__name__)) class Option(BaseInformation): @@ -150,7 +150,7 @@ class Option(BaseInformation): except ValueError, err: raise ValueError(_("invalid default_multi value {0} " "for option {1}: {2}").format( - str(default_multi), name, err)) + str(default_multi), name, err)) if callback is not None and (default is not None or default_multi is not None): raise ValueError(_("default value not allowed if option: {0} " @@ -180,8 +180,8 @@ class Option(BaseInformation): if not isinstance(properties, tuple): raise TypeError(_('invalid properties type {0} for {1},' ' must be a tuple').format( - type(properties), - self._name)) + type(properties), + self._name)) self._properties = properties # 'hidden', 'disabled'... def __eq__(self, other): @@ -329,7 +329,7 @@ class Option(BaseInformation): "no default value has been set yet" if ((not self.impl_is_multi() and self._default is None) or (self.impl_is_multi() and (self._default == [] - or None in self._default))): + or None in self._default))): return True return False @@ -1003,10 +1003,7 @@ def validate_requires_arg(requires, name): ret_action = [] for require in opt_requires.values(): req = (require[0], tuple(require[1]), - require[2], - require[3], - require[4], - require[5]) + require[2], require[3], require[4], require[5]) ret_action.append(req) ret.append(tuple(ret_action)) return frozenset(config_action.keys()), tuple(ret) From 776524a22d0c1537462226b61558568526492153 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Wed, 28 Aug 2013 09:18:48 +0200 Subject: [PATCH 6/6] pep8 --- tiramisu/option.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tiramisu/option.py b/tiramisu/option.py index 31f8cc8..724c691 100644 --- a/tiramisu/option.py +++ b/tiramisu/option.py @@ -1002,8 +1002,8 @@ def validate_requires_arg(requires, name): for opt_requires in ret_requires.values(): ret_action = [] for require in opt_requires.values(): - req = (require[0], tuple(require[1]), - require[2], require[3], require[4], require[5]) + req = (require[0], tuple(require[1]), require[2], require[3], + require[4], require[5]) ret_action.append(req) ret.append(tuple(ret_action)) return frozenset(config_action.keys()), tuple(ret)