From f6073a3ddfc74fdb6111a49a8eb6620fa4ff9b08 Mon Sep 17 00:00:00 2001 From: Shay Palachi Date: Tue, 24 Nov 2020 23:09:34 +0200 Subject: [PATCH] working on memory core locking + tests Memory core is now thread safe extend memory core tests added checkdocs --- .fdignore | 3 + .pylintrc | 275 ++++++++++++++++++++++++++++++++++++++ README.rst | 26 +++- cachier/core.py | 2 +- cachier/memory_core.py | 88 ++++++++---- cachier/pickle_core.py | 4 +- pytest.ini | 2 + setup.py | 11 +- tests/test_memory_core.py | 59 ++++++-- 9 files changed, 427 insertions(+), 43 deletions(-) create mode 100644 .fdignore create mode 100644 .pylintrc diff --git a/.fdignore b/.fdignore new file mode 100644 index 0000000..c7bf753 --- /dev/null +++ b/.fdignore @@ -0,0 +1,3 @@ +.venv +.pytest_cache +notebooks diff --git a/.pylintrc b/.pylintrc new file mode 100644 index 0000000..ebd4427 --- /dev/null +++ b/.pylintrc @@ -0,0 +1,275 @@ +[MASTER] + +# Specify a configuration file. +#rcfile= + +# Python code to execute, usually for sys.path manipulation such as +# pygtk.require(). +#init-hook= + +# Profiled execution. +profile=no + +# Add files or directories to the blacklist. They should be base names, not +# paths. +ignore=CVS + +# Pickle collected data for later comparisons. +persistent=yes + +# List of plugins (as comma separated values of python modules names) to load, +# usually to register additional checkers. +load-plugins= + + +[MESSAGES CONTROL] + +# Enable the message, report, category or checker with the given id(s). You can +# either give multiple identifier separated by comma (,) or put this option +# multiple time. See also the "--disable" option for examples. +#enable= + +# Disable the message, report, category or checker with the given id(s). You +# can either give multiple identifiers separated by comma (,) or put this +# option multiple times (only on the command line, not in the configuration +# file where it should appear only once).You can also use "--disable=all" to +# disable everything first and then reenable specific checks. For example, if +# you want to run only the similarities checker, you can use "--disable=all +# --enable=similarities". If you want to run only the classes checker, but have +# no Warning level messages displayed, use"--disable=all --enable=classes +# --disable=W" +disable=F0401,W0141,W0611,W0110,W0703,W0142 +disable=W0603 +disable=E1123 + + +[REPORTS] + +# Set the output format. Available formats are text, parseable, colorized, msvs +# (visual studio) and html. You can also give a reporter class, eg +# mypackage.mymodule.MyReporterClass. +output-format=text + +# Put messages in a separate file for each module / package specified on the +# command line instead of printing them on stdout. Reports (if any) will be +# written in a file name "pylint_global.[txt|html]". +files-output=no + +# Tells whether to display a full report or only the messages +reports=yes + +# Python expression which should return a note less than 10 (10 is the highest +# note). You have access to the variables errors warning, statement which +# respectively contain the number of errors / warnings messages and the total +# number of statements analyzed. This is used by the global evaluation report +# (RP0004). +evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10) + +# Add a comment according to your evaluation note. This is used by the global +# evaluation report (RP0004). +comment=no + +# Template used to display messages. This is a python new-style format string +# used to format the massage information. See doc for all details +#msg-template= + + +[BASIC] + +# Required attributes for module, separated by a comma +required-attributes= + +# List of builtins function names that should not be used, separated by a comma +bad-functions=map,filter,apply,input + +# Regular expression which should only match correct module names +module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ + +# Regular expression which should only match correct module level names +const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__))$ + +# Regular expression which should only match correct class names +class-rgx=[A-Z_][a-zA-Z0-9]+$ + +# Regular expression which should only match correct function names +function-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct method names +method-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct instance attribute names +attr-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct argument names +argument-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct variable names +variable-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match correct attribute names in class +# bodies +class-attribute-rgx=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$ + +# Regular expression which should only match correct list comprehension / +# generator expression variable names +inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$ + +# Good variable names which should always be accepted, separated by a comma +good-names=i,j,k,ex,Run,_ + +# Bad variable names which should always be refused, separated by a comma +bad-names=foo,bar,baz,toto,tutu,tata + +# Regular expression which should only match function or class names that do +# not require a docstring. +no-docstring-rgx=__.*__ + +# Minimum line length for functions/classes that require docstrings, shorter +# ones are exempt. +docstring-min-length=-1 + + +[VARIABLES] + +# Tells whether we should check for unused import in __init__ files. +init-import=no + +# A regular expression matching the beginning of the name of dummy variables +# (i.e. not used). +dummy-variables-rgx=_|dummy + +# List of additional names supposed to be defined in builtins. Remember that +# you should avoid to define new builtins when possible. +additional-builtins= + + +[SIMILARITIES] + +# Minimum lines number of a similarity. +min-similarity-lines=4 + +# Ignore comments when computing similarities. +ignore-comments=yes + +# Ignore docstrings when computing similarities. +ignore-docstrings=yes + +# Ignore imports when computing similarities. +ignore-imports=no + + +[MISCELLANEOUS] + +# List of note tags to take in consideration, separated by a comma. +notes=FIXME,XXX,TODO + + +[FORMAT] + +# Maximum number of characters on a single line. +max-line-length=80 + +# Regexp for a line that is allowed to be longer than the limit. +ignore-long-lines=^\s*(# )??$ + +# Maximum number of lines in a module +max-module-lines=1000 + +# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 +# tab). +indent-string=' ' + + +[TYPECHECK] + +# Tells whether missing members accessed in mixin class should be ignored. A +# mixin class is detected if its name ends with "mixin" (case insensitive). +ignore-mixin-members=yes + +# List of classes names for which member attributes should not be checked +# (useful for classes with attributes dynamically set). +ignored-classes=SQLObject + +# When zope mode is activated, add a predefined set of Zope acquired attributes +# to generated-members. +zope=no + +# List of members which are set dynamically and missed by pylint inference +# system, and so shouldn't trigger E0201 when accessed. Python regular +# expressions are accepted. +generated-members=REQUEST,acl_users,aq_parent + + +[IMPORTS] + +# Deprecated modules which should not be used, separated by a comma +deprecated-modules=regsub,TERMIOS,Bastion,rexec + +# Create a graph of every (i.e. internal and external) dependencies in the +# given file (report RP0402 must not be disabled) +import-graph= + +# Create a graph of external dependencies in the given file (report RP0402 must +# not be disabled) +ext-import-graph= + +# Create a graph of internal dependencies in the given file (report RP0402 must +# not be disabled) +int-import-graph= + + +[DESIGN] + +# Maximum number of arguments for function / method +max-args=5 + +# Argument names that match this expression will be ignored. Default to name +# with leading underscore +ignored-argument-names=_.* + +# Maximum number of locals for function / method body +max-locals=15 + +# Maximum number of return / yield for function / method body +max-returns=6 + +# Maximum number of branch for function / method body +max-branches=12 + +# Maximum number of statements in function / method body +max-statements=50 + +# Maximum number of parents for a class (see R0901). +max-parents=7 + +# Maximum number of attributes for a class (see R0902). +max-attributes=7 + +# Minimum number of public methods for a class (see R0903). +min-public-methods=2 + +# Maximum number of public methods for a class (see R0904). +max-public-methods=20 + + +[CLASSES] + +# List of interface methods to ignore, separated by a comma. This is used for +# instance to not check methods defines in Zope's Interface base class. +ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by + +# List of method names used to declare (i.e. assign) instance attributes. +defining-attr-methods=__init__,__new__,setUp + +# List of valid names for the first argument in a class method. +valid-classmethod-first-arg=cls + +# List of valid names for the first argument in a metaclass class method. +valid-metaclass-classmethod-first-arg=mcs + + +[EXCEPTIONS] + +# Exceptions that will emit a warning when being caught. Defaults to +# "Exception" +overgeneral-exceptions=Exception diff --git a/README.rst b/README.rst index b8a5a63..d276ad5 100644 --- a/README.rst +++ b/README.rst @@ -214,6 +214,18 @@ In certain cases the MongoDB backend might leave a deadlock behind, blocking all @cachier(mongetter=False, wait_for_calc_timeout=2) +Memory Core +----------- + +You can set an in-memory cache by assigning the ``backend`` parameter with ``'memory'``: + +.. code-block:: python + + @cachier(backend='memory') + +Note, however, that ``cachier``'s in-memory core is simple, and has no monitoring or cap on cache size, and can thus lead to memory errors on large return values - it is mainly intended to be used with future multi-core functionality. As a rule, Python's built-in ``lru_cache`` is a much better stand-alone solution. + + Contributing ============ @@ -255,10 +267,22 @@ This project is documented using the `numpy docstring conventions`_, which were .. _`numpy docstring conventions`: https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt .. _`these conventions`: https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt +Additionally, if you update this ``README.rst`` file, use ``python setup.py checkdocs`` to validate it compiles. + Credits ======= -Created by Shay Palachy (shay.palachy@gmail.com). +Created by `Shay Palachy `_ (shay.palachy@gmail.com). + +Other major contributors: + + * `cthoyt `_ - Base memory core implementation. + + * `amarczew `_ - The ``hash_params`` kwarg. + + * `non-senses `_ - The ``wait_for_calc_timeout`` kwarg. + +Notable bugfixers: `MichaelRazum `_, .. Contributers (in chronological order of first commit): diff --git a/cachier/core.py b/cachier/core.py index baf1a41..3251311 100644 --- a/cachier/core.py +++ b/cachier/core.py @@ -119,7 +119,7 @@ def cachier( backend : str, optional The name of the backend to use. If None, defaults to 'mongo' when the ``mongetter`` argument is passed, otherwise defaults to 'pickle'. - Valid options currently include 'pickle' and 'mongo'. + Valid options currently include 'pickle', 'mongo' and 'memory'. cache_dir : str, optional A fully qualified path to a file directory to be used for cache files. The running process must have running permissions to this folder. If diff --git a/cachier/memory_core.py b/cachier/memory_core.py index fc15f51..b8442f3 100644 --- a/cachier/memory_core.py +++ b/cachier/memory_core.py @@ -1,6 +1,6 @@ """A memory-based caching core for cachier.""" -from collections import defaultdict +import threading from datetime import datetime from .base_core import _BaseCore @@ -20,48 +20,80 @@ class _MemoryCore(_BaseCore): def __init__(self, stale_after, next_time): super().__init__(stale_after=stale_after, next_time=next_time) self.cache = {} + self.lock = threading.RLock() def get_entry_by_key(self, key, reload=False): # pylint: disable=W0221 - return key, self.cache.get(key, None) + with self.lock: + return key, self.cache.get(key, None) def get_entry(self, args, kwds, hash_params): - key = args + tuple(sorted(kwds.items())) if hash_params is None else hash_params(args, kwds) - return self.get_entry_by_key(key) + with self.lock: + key = args + tuple(sorted(kwds.items())) if hash_params is None else hash_params(args, kwds) # noqa: E501 + return self.get_entry_by_key(key) def set_entry(self, key, func_res): - self.cache[key] = { - 'value': func_res, - 'time': datetime.now(), - 'stale': False, - 'being_calculated': False, - } - - def mark_entry_being_calculated(self, key): - try: - self.cache[key]['being_calculated'] = True - except KeyError: + with self.lock: + try: + # we need to retain the existing condition so that + # mark_entry_not_calculated can notify all possibly-waiting + # threads about it + cond = self.cache[key]['condition'] + except KeyError: # pragma: no cover + cond = None self.cache[key] = { - 'value': None, + 'value': func_res, 'time': datetime.now(), 'stale': False, - 'being_calculated': True, + 'being_calculated': False, + 'condition': cond, } + def mark_entry_being_calculated(self, key): + with self.lock: + condition = threading.Condition() + # condition.acquire() + try: + self.cache[key]['being_calculated'] = True + self.cache[key]['condition'] = condition + except KeyError: + self.cache[key] = { + 'value': None, + 'time': datetime.now(), + 'stale': False, + 'being_calculated': True, + 'condition': condition, + } + def mark_entry_not_calculated(self, key): - try: - self.cache[key]['being_calculated'] = False - except KeyError: - pass # that's ok, we don't need an entry in that case + with self.lock: + try: + entry = self.cache[key] + except KeyError: # pragma: no cover + return # that's ok, we don't need an entry in that case + entry['being_calculated'] = False + cond = entry['condition'] + if cond: + cond.acquire() + cond.notify_all() + cond.release() + entry['condition'] = None def wait_on_entry_calc(self, key): - entry = self.cache[key] - # I don't think waiting is necessary for this one - # if not entry['being_calculated']: - return entry['value'] + with self.lock: + entry = self.cache[key] + if not entry['being_calculated']: + return entry['value'] # pragma: no cover + entry['condition'].acquire() + entry['condition'].wait() + entry['condition'].release() + return self.cache[key]['value'] def clear_cache(self): - self.cache.clear() + with self.lock: + self.cache.clear() def clear_being_calculated(self): - for value in self.cache.values(): - value['being_calculated'] = False + with self.lock: + for entry in self.cache.values(): + entry['being_calculated'] = False + entry['condition'] = None diff --git a/cachier/pickle_core.py b/cachier/pickle_core.py index 142dd17..912ba03 100644 --- a/cachier/pickle_core.py +++ b/cachier/pickle_core.py @@ -76,9 +76,11 @@ class _PickleCore(_BaseCore): self.observer.stop() def on_created(self, event): # skipcq: PYL-W0613 + """A Watchdog Event Handler method.""" self._check_calculation() # pragma: no cover def on_modified(self, event): # skipcq: PYL-W0613 + """A Watchdog Event Handler method.""" self._check_calculation() def __init__(self, stale_after, next_time, reload, cache_dir): @@ -147,7 +149,7 @@ class _PickleCore(_BaseCore): return key, self._get_cache().get(key, None) def get_entry(self, args, kwds, hash_params): - key = args + tuple(sorted(kwds.items())) if hash_params is None else hash_params(args, kwds) + key = args + tuple(sorted(kwds.items())) if hash_params is None else hash_params(args, kwds) # noqa: E501 # print('key type={}, key={}'.format(type(key), key)) return self.get_entry_by_key(key) diff --git a/pytest.ini b/pytest.ini index 09575d5..bfd62dc 100644 --- a/pytest.ini +++ b/pytest.ini @@ -3,6 +3,8 @@ testpaths = tests cachier norecursedirs=dist build .tox +markers = + memory: test memory core functionalities. addopts = # --doctest-modules --cov=cachier diff --git a/setup.py b/setup.py index 1bdd079..a1d4d21 100644 --- a/setup.py +++ b/setup.py @@ -15,7 +15,16 @@ except ImportError: import versioneer -TEST_REQUIRES = ['pytest', 'coverage', 'pytest-cov', 'pymongo', 'pandas'] +TEST_REQUIRES = [ + # testing and coverage + 'pytest', 'coverage', 'pytest-cov', + # optional dependencies + 'pymongo', + # non-testing packages required by tests, not by the package + 'pandas', + # to be able to run `python setup.py checkdocs` + 'collective.checkdocs', 'pygments', +] README_RST = '' with open('README.rst') as f: diff --git a/tests/test_memory_core.py b/tests/test_memory_core.py index a63358f..244c3b3 100644 --- a/tests/test_memory_core.py +++ b/tests/test_memory_core.py @@ -7,27 +7,29 @@ from datetime import timedelta from random import random from time import sleep, time +import pytest import pandas as pd from cachier import cachier @cachier(backend='memory', next_time=False) -def _takes_5_seconds(arg_1, arg_2): +def _takes_2_seconds(arg_1, arg_2): """Some function.""" - sleep(5) + sleep(2) return 'arg_1:{}, arg_2:{}'.format(arg_1, arg_2) +@pytest.mark.memory def test_memory_core(): """Basic memory core functionality.""" - _takes_5_seconds.clear_cache() - _takes_5_seconds('a', 'b') + _takes_2_seconds.clear_cache() + _takes_2_seconds('a', 'b') start = time() - _takes_5_seconds('a', 'b', verbose_cache=True) + _takes_2_seconds('a', 'b', verbose_cache=True) end = time() assert end - start < 1 - _takes_5_seconds.clear_cache() + _takes_2_seconds.clear_cache() SECONDS_IN_DELTA = 3 @@ -40,6 +42,7 @@ def _stale_after_seconds(arg_1, arg_2): return random() +@pytest.mark.memory def test_stale_after(): """Testing the stale_after functionality.""" _stale_after_seconds.clear_cache() @@ -60,6 +63,7 @@ def _stale_after_next_time(arg_1, arg_2): return random() +@pytest.mark.memory def test_stale_after_next_time(): """Testing the stale_after with next_time functionality.""" _stale_after_next_time.clear_cache() @@ -88,6 +92,7 @@ def _random_num_with_arg(a): return random() +@pytest.mark.memory def test_overwrite_cache(): """Tests that the overwrite feature works correctly.""" _random_num.clear_cache() @@ -111,6 +116,7 @@ def test_overwrite_cache(): _random_num_with_arg.clear_cache() +@pytest.mark.memory def test_ignore_cache(): """Tests that the ignore_cache feature works correctly.""" _random_num.clear_cache() @@ -148,12 +154,15 @@ def _calls_takes_time(res_queue): res_queue.put(res) +@pytest.mark.memory def test_memory_being_calculated(): """Testing memory core handling of being calculated scenarios.""" _takes_time.clear_cache() res_queue = queue.Queue() - thread1 = threading.Thread(target=_calls_takes_time, kwargs={'res_queue': res_queue}) - thread2 = threading.Thread(target=_calls_takes_time, kwargs={'res_queue': res_queue}) + thread1 = threading.Thread( + target=_calls_takes_time, kwargs={'res_queue': res_queue}) + thread2 = threading.Thread( + target=_calls_takes_time, kwargs={'res_queue': res_queue}) thread1.start() sleep(0.5) thread2.start() @@ -177,6 +186,7 @@ def _calls_being_calc_next_time(res_queue): res_queue.put(res) +@pytest.mark.memory def test_being_calc_next_time(): """Testing memory core handling of being calculated scenarios.""" _takes_time.clear_cache() @@ -212,8 +222,31 @@ def _delete_cache(arg_1, arg_2): return random() + arg_1 + arg_2 +@pytest.mark.memory def test_clear_being_calculated(): """Test memory core clear `being calculated` functionality.""" + _takes_time.clear_cache() + res_queue = queue.Queue() + thread1 = threading.Thread( + target=_calls_takes_time, kwargs={'res_queue': res_queue}) + thread2 = threading.Thread( + target=_calls_takes_time, kwargs={'res_queue': res_queue}) + thread1.start() + _takes_time.clear_being_calculated() + sleep(0.5) + thread2.start() + thread1.join() + thread2.join() + assert res_queue.qsize() == 2 + res1 = res_queue.get() + res2 = res_queue.get() + assert res1 != res2 + + +@pytest.mark.memory +def test_clear_being_calculated_with_empty_cache(): + """Test memory core clear `being calculated` functionality.""" + _takes_time.clear_cache() _takes_time.clear_being_calculated() @@ -227,6 +260,7 @@ def _error_throwing_func(arg1): return 7 +@pytest.mark.memory def test_error_throwing_func(): # with res1 = _error_throwing_func(4) @@ -235,15 +269,18 @@ def test_error_throwing_func(): assert res1 == res2 +@pytest.mark.memory def test_callable_hash_param(): def _hash_params(args, kwargs): def _hash(obj): if isinstance(obj, pd.core.frame.DataFrame): - return hashlib.sha256(pd.util.hash_pandas_object(obj).values.tobytes()).hexdigest() + return hashlib.sha256( + pd.util.hash_pandas_object(obj).values.tobytes() + ).hexdigest() return obj - k_args = tuple(map(_hash, args)) - k_kwargs = tuple(sorted({k: _hash(v) for k, v in kwargs.items()}.items())) + k_kwargs = tuple(sorted( + {k: _hash(v) for k, v in kwargs.items()}.items())) return k_args + k_kwargs @cachier(backend='memory', hash_params=_hash_params)