[Predator] Add local cache for get command output.

appengine/findit/common/cache_decorator.py

Index: appengine/findit/common/cache_decorator.py
diff --git a/appengine/findit/common/cache_decorator.py b/appengine/findit/common/cache_decorator.py
index 6f1bb1efe42dc866aa7841e4df8eb84db698f26f..3e76b477af1f117894d7aaa10ab8560a24ba413a 100644
--- a/appengine/findit/common/cache_decorator.py
+++ b/appengine/findit/common/cache_decorator.py
@@ -39,11 +39,15 @@ import functools
 import hashlib
 import inspect
 import logging
+import os
 import pickle
+import threading
 import zlib
 
 from google.appengine.api import memcache
 
+_DEFAULT_LOCAL_CACHE_DIR = os.path.join(os.path.expanduser('~'), '.cache')

[Martin Barbella, 2016/10/26 23:21:20]

This shouldn't use such a generic name. This will actually already exist and be
used for other things in many environments. Maybe something like
~/.predator/cache.

[wrengr, 2016/10/27 16:36:47]

Seconded.

[Sharu Jiang, 2016/10/27 21:59:05]

Done. Use .culprit_finder/local_cache since it's shared code in common.

+
 
 class Cacher(object):

[wrengr, 2016/10/27 16:36:47]

Using "-er" names for classes is weird. Why not just call it "Cache"?

[Sharu Jiang, 2016/10/27 21:59:05]

I think it means 'Cacher' caches.

[wrengr, 2016/10/28 18:02:00]

In all the communities I've programmed in, we don't subjectize nouns. The base
noun is more natural in English. (Just like how we don't call it a "collector"
we call it a "collection"; we don't call it a "storer" we call it "storage"; we
don't call it an "adder" (outside of circuitry) we call it "addition"; we don't
call it a "runner" we call it a "task", or "thread", or "pipeline", or whatever;
etc.) There are only a handful of situations where the "-er" form is used:
namely where the computational object has taken on a life of its own, outside
the the results it produces. For only then do we start thinking about the
producer of results as an entity which stands on its own and deserves more focus
than the results it produces. The act of caching is very simple, so using "-er"
puts emphasis on the wrong thing.

"Fetcher" also sounds similarly alien, fwiw.

[Sharu Jiang, 2016/11/08 01:17:12]

This make senses, added a todo.

   """An interface to cache and retrieve data.
@@ -133,6 +137,38 @@ class CompressedMemCacher(Cacher):
     return len(keys_not_set) == 0
 
 
+class LocalCacher(object):

[Martin Barbella, 2016/10/26 23:21:20]

Solely for the sake of consistency, it seems like this should extend Cacher.

[Sharu Jiang, 2016/10/27 21:59:05]

Oops, it should.

+  """Cacher that uses local files to cache data."""
+
+  lock = threading.Lock()
+
+  def __init__(self, cache_dir=_DEFAULT_LOCAL_CACHE_DIR):
+    self.cache_dir = cache_dir
+    if not os.path.exists(cache_dir):  # pragma: no cover.
+      os.mkdir(cache_dir)
+
+  def Get(self, key):
+    with LocalCacher.lock:
+      path = os.path.join(self.cache_dir, key)
+      if not os.path.exists(path):
+        return None
+
+      try:
+        with open(path) as f:
+          return pickle.loads(zlib.decompress(f.read()))
+      except Exception:  # pragma: no cover.
+        logging.error('Failed load cache.')

[Martin Barbella, 2016/10/26 23:21:20]

Could you make this error a bit more descriptive, similar to the one below?

[Sharu Jiang, 2016/10/27 21:59:05]

Done.

+        return None
+
+  def Set(self, key, data):  # pylint: disable=W
+    with LocalCacher.lock:
+      try:
+        with open(os.path.join(self.cache_dir, key), 'wb') as f:
+          f.write(zlib.compress(pickle.dumps(data)))
+      except Exception:  # pragma: no cover.
+        logging.error('Failed to cache key %s', key)
+
+
 def _DefaultKeyGenerator(func, args, kwargs):
   """Generates a key from the function and arguments passed to it.