Issue 395 - Filter hits statistics backend

sitescripts/filterhits/bin/process_logs.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
-# Copyright (C) 2006-2014 Eyeo GmbH
+# Copyright (C) 2006-2015 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import MySQLdb, itertools, json, os, sys
 from sitescripts.utils import get_config
 import sitescripts.filterhits.common as common
 import sitescripts.filterhits.db as db
 import sitescripts.filterhits.geometrical_mean as geometrical_mean
 
 last_log_file = None
 
 def log_files(dir):
   """
-   Provides a generator of filter hits log files for the given directory.
-   Works recursively, relative path of log file is returned.
+  Provides a generator of filter hits log files for the given directory.
+  Works recursively, relative path of log file is returned.
   """
   for root, subdirs, files in os.walk(dir):
     for f in files:
-      if f.endswith(".log") and f[0].isdigit():
+      if os.path.splitext(f)[1] == ".log" and f[0].isdigit():
         yield os.path.join(root, f)
 
 def read_data(log_file):
   """
-   Read, parse and return the JSON data for the given log file name.
-   (As a side effect sets the global last_log_file to the log file name.)
+  Read, parse and return the JSON data for the given log file name.
+  (As a side effect sets the global last_log_file to the log file name.)
   """
   global last_log_file
   try:
     with open(log_file, "r") as f:
       # Skip past the date and GET parameters
-      s = ""
-      while s != "\" ":
-        s = s[-1:] + f.read(1)
+      current = last = None
+      while not (last == '"' and current == " "):

[Sebastian Noack, 2015/02/17 14:59:17]

If you use the != instead the == operator in the first place, you don't need to
negate the result:

last != '"' or current != ' '

[kzar, 2015/02/24 18:05:11]

I'm aware of Demorgan's law but I think the intention is much clearer when it's
written this way around.

[Sebastian Noack, 2015/02/26 16:39:25]

I'd rather say, as more logical operations involved, as harder it's to
understand the logic.

[kzar, 2015/02/28 19:39:56]

Well what we're saying is something like "While the last two characters aren't
'" ' then do this...". I think `not (last == '"' and current == " ")` is closer
to that than `last != '"' or current != ' '` but I guess it's subjective.

How about this? It's even closer to the English version and reduces the logical
operations.

[Sebastian Noack, 2015/03/02 10:04:01]

Not sure whether I like it, but fair enough.

+        last, current = current, f.read(1)
+        if not current:
+          sys.exit("Unexpected EOF in log file %s" % log_file)
+
       # Read the JSON
       data = json.load(f)
       # Keep track of the current log file in global variable in case we need to
       # identify it later if there's a problem. (This works because the files are
       # processed lazily.)
       last_log_file = log_file
   except IOError:
     sys.exit("Could not read log file %s" % log_file)
-  if not common.valid_log_data(data):
-    sys.exit("Invalid data in log file %s." % log_file)
   return data
 
 if __name__ == "__main__":
   if not len(sys.argv) == 2:
     print "Usage: python -m sitescripts.filterhits.bin.process_logs /path/to/logs"
     sys.exit(1)
 
   config = get_config()
   interval = config.get("filterhitstats", "interval")
 
   def read_update(f):
     return geometrical_mean.update(interval, read_data(f))
 
   if sys.argv[1].endswith(".log"):
     sql = read_update(sys.argv[1])
   else:
     sql = itertools.chain.from_iterable(itertools.imap(read_update,
                                                        log_files(sys.argv[1])))
 
   try:
-    db.connect(config.get("filterhitstats", "dbuser"),
-               config.get("filterhitstats", "dbpassword"),
-               config.get("filterhitstats", "database"))
-    db.write(sql)
+    db_connection = db.connect(config.get("filterhitstats", "dbuser"),
+                               config.get("filterhitstats", "dbpassword"),
+                               config.get("filterhitstats", "database"))
+    db.write(db_connection, sql)
   except MySQLdb.Error, e:
     sys.exit("Failed to process file %s, all changes rolled back. MySQl error (%d): \"%s\"\n" % (
       last_log_file, e.args[0], e.args[1]
     ))
   finally:
-    db.disconnect()
+    if db_connection:
+      db_connection.close()