Issue 2983 - Make sure that the new lastVersion format is recognized and exceptions don`t break eve…

sitescripts/stats/bin/logprocessor.py

Index: sitescripts/stats/bin/logprocessor.py
===================================================================
--- a/sitescripts/stats/bin/logprocessor.py
+++ b/sitescripts/stats/bin/logprocessor.py
@@ -112,17 +112,21 @@ def cache_lru(func):
     if arg in results:
       result = results[arg]
       del results[arg]
     else:
       if results.entries_left > 0:
         results.entries_left -= 1
       else:
         results.popitem(last=False)
-      result = func(arg)
+      try:
+        result = func(arg)
+      except:

[Sebastian Noack, 2015/10/21 15:19:20]

You probably should explicitly catch Exception. Otherwise that code will catch
SystemExit and KeyboardInterrupt as well.

[Wladimir Palant, 2015/10/21 15:42:40]

Why shouldn't it? The exception is re-raised, the point is merely keeping the
cache in a consistent state. Whether some exception will cause the interpreter
to shut down is irrelevant for that.

[Sebastian Noack, 2015/10/21 15:49:13]

Acknowledged.

+        results.entries_left += 1
+        raise
     results[arg] = result
     return result
   return wrapped
 
 
 def cache_last(func):
   """
     Decorator that memoizes the last return value of a function in case it is
@@ -262,16 +266,18 @@ def parse_path(path):
   return path[1:], urlparts.query
 
 @cache_lru
 def parse_query(query):
   return urlparse.parse_qs(query)
 
 @cache_lru
 def parse_lastversion(last_version):
+  if '-' in last_version:
+    last_version = last_version.split('-', 1)[0]

[Sebastian Noack, 2015/10/21 15:19:20]

Nit: last_version.split('-')[0] would do as will and is slightly simpler.

[Wladimir Palant, 2015/10/21 15:42:40]

Sure, but the point is doing as little work as possible here.

[Sebastian Noack, 2015/10/21 15:49:13]

More than one dash given is a rather theoretical scenario, we don't have to
optimize for, right? Because if there is only one dash given, split() is even
faster when omitting the second argument:

In [3]: %timeit '0-1'.split('-')
1000000 loops, best of 3: 180 ns per loop

In [4]: %timeit '0-1'.split('-', 1)
1000000 loops, best of 3: 192 ns per loop

[Sebastian Noack, 2015/10/21 16:10:34]

I did some more benchmarking, and confirmed that the |'-' in last_version| check
is indeed giving some notable speedups if there is no dash. However, if the
common case is a dash being given, omitting that check is obviously faster (and
also simpler):

In [10]: x = '0-1'

In [11]: %timeit x.split(x, 1)[0] if '-' in x else x
1000000 loops, best of 3: 243 ns per loop

In [12]: %timeit x.split(x)[0]
1000000 loops, best of 3: 204 ns per loop

In [13]: x = '0'

In [14]: %timeit x.split(x, 1)[0] if '-' in x else x
10000000 loops, best of 3: 38.1 ns per loop

In [15]: %timeit x.split(x)[0]
1000000 loops, best of 3: 197 ns per loop

[Wladimir Palant, 2015/10/21 16:16:03]

No, it's a very real scenario if more than one notification is active at a time.
As is the "no dash" scenario (no notifications active).

Either way, should we really argue about microseconds? The point is that we only
need one split here.

[Sebastian Noack, 2015/10/21 16:19:51]

Well, I'm not arguing. I just try to understand whether the added complexity is
worth the few microseconds you save in these scenarios. LGTM.

   return datetime.strptime(last_version, "%Y%m%d%H%M")
 
 @cache_lru
 def get_week(date):
   return date.isocalendar()[0:2]
 
 def parse_downloader_query(info):
   params = parse_query(info["query"])