Merge lp:~sylvain-pineau/checkbox/whitelist_ordering_with_job_cache into lp:checkbox

[Sylvain Pineau]

This branch brings two new features. A JobStore cache mechanism (thanks to Daniel Manrique) but entirely handled in message.py and the whitelist ordering proposal V2.0 (let's consider lp:~oem-qa/checkbox/patch_selection_dialog_sooner_and_whitelist_ordering as my first attempt).

This time, dependency checking is performed automatically and dependency jobs run in the whitelist order.

If errors are detected in the whitelist file or in jobs properties (doubled values, badly ordered dependencies, dependencies not found) they are all logged into the checkbox log file as warnings.

[lp:~sylvain-pineau/checkbox/whitelist_ordering_with_job_cache]

1247. By Sylvain Pineau on 2012-02-15

Whitelist ordering

[Sylvain Pineau (sylvain-pineau)]

The ordering is now performed in plugins/jobs_info.py, much better.
Dependencies are ordered thanks to the JobStore.add call.
The ordering itself now relies on a simple cmp call, thanks cr3.

review: Needs Resubmitting

[Marc Tardif (cr3)]

Looks good, thanks!

review: Approve

=== modified file 'checkbox/job.py'
--- checkbox/job.py	2011-09-28 20:46:04 +0000
+++ checkbox/job.py	2012-02-15 17:36:58 +0000
@@ -122,7 +122,7 @@
         if "depends" in job:
             for depends in job["depends"]:
                 for filename in self._find_matching_messages():
-                    message = self._read_message(filename)
+                    message = self._read_message(filename, cache=True)
                     if job["name"] in message.get("depends", []):
                         new_filename = self._get_next_message_filename()
                         os.rename(filename, new_filename)
@@ -132,7 +132,7 @@
     # TODO: Optimize by only searching backwards until a given condition
     def _find_matching_messages(self, **kwargs):
         for filename in self._walk_messages():
-            message = self._read_message(filename)
+            message = self._read_message(filename,cache=True)
             for key, value in kwargs.iteritems():
                 if message.get(key) != value:
                     break

=== modified file 'checkbox/message.py'
--- checkbox/message.py	2011-10-07 13:07:16 +0000
+++ checkbox/message.py	2012-02-15 17:36:58 +0000
@@ -20,6 +20,7 @@
 import logging
 import itertools
 import posixpath
+import sys
 
 from checkbox.contrib import bpickle
 from checkbox.lib.safe import safe_close
@@ -40,6 +41,9 @@
 class MessageStore(object):
     """A message store which stores its messages in a file system hierarchy."""
 
+    #This caches everything but a message's data, making it manageable to keep in memory.
+    _message_cache = {}
+
     def __init__(self, persist, directory, directory_size=1000):
         self._directory = directory
         self._directory_size = directory_size
@@ -240,7 +244,11 @@
     def _dump_message(self, message):
         return bpickle.dumps(message)
 
-    def _read_message(self, filename):
+    def _read_message(self, filename, cache=False):
+        #cache basically indicates whether the caller cares about having "data"
+        if cache and filename in self._message_cache:
+            return Message(self._message_cache[filename],filename)
+
         data = self._get_content(filename)
         message = self._load_message(data)
         return Message(message, filename)
@@ -257,6 +265,14 @@
 
         os.rename(filename + ".tmp", filename)
 
+        #Strip the big data element and shove it in the cache
+
+        temp_message=dict(message)
+        if "data" in temp_message:
+            temp_message["data"] = None
+
+        self._message_cache[filename] = temp_message
+
         # For now we use the inode as the message id, as it will work
         # correctly even faced with holding/unholding.  It will break
         # if the store is copied over for some reason, but this shouldn't

=== modified file 'checkbox/user_interface.py'
--- checkbox/user_interface.py	2012-02-13 01:44:19 +0000
+++ checkbox/user_interface.py	2012-02-15 17:36:58 +0000
@@ -27,8 +27,6 @@
 import gettext
 from gettext import gettext as _
 
-from gi.repository import Gio
-
 from checkbox.contrib.REThread import REThread
 
 from checkbox.lib.environ import add_variable, get_variable, remove_variable
@@ -180,6 +178,8 @@
                 if os.getenv("DISPLAY") and \
                         subprocess.call(["pgrep", "-x", "-u", str(uid), "gnome-panel|gconfd-2"],
                                 stdout=subprocess.PIPE, stderr=subprocess.PIPE) == 0:
+                    from gi.repository import Gio
+
                     preferred_xml_app = Gio.app_info_get_default_for_type("application/xml",False)
                     if preferred_xml_app:
                         preferred_browser = preferred_xml_app.get_executable()

=== modified file 'plugins/jobs_info.py'
--- plugins/jobs_info.py	2012-02-07 18:30:21 +0000
+++ plugins/jobs_info.py	2012-02-15 17:36:58 +0000
@@ -54,16 +54,16 @@
 
     # Space separated list of directories where job files are stored.
     directories = List(Path(),
-        default_factory=lambda:"%(checkbox_share)s/jobs")
+        default_factory=lambda: "%(checkbox_share)s/jobs")
 
     # List of jobs to blacklist
-    blacklist = List(String(), default_factory=lambda:"")
+    blacklist = List(String(), default_factory=lambda: "")
 
     # Path to blacklist file
     blacklist_file = Path(required=False)
 
     # List of jobs to whitelist
-    whitelist = List(String(), default_factory=lambda:"")
+    whitelist = List(String(), default_factory=lambda: "")
 
     # Path to whitelist file
     whitelist_file = Path(required=False)
@@ -82,8 +82,8 @@
             try:
                 file = open(filename)
             except IOError, e:
-                error_message=(gettext.gettext("Failed to open file '%s': %s") % 
-                        (filename,e.strerror))
+                error_message=(gettext.gettext("Failed to open file '%s': %s") %
+                        (filename, e.strerror))
                 logging.critical(error_message)
                 sys.stderr.write("%s\n" % error_message)
                 sys.exit(os.EX_NOINPUT)
@@ -95,8 +95,15 @@
 
     def gather(self):
         # Register temporary handler for report-message events
+        messages = []
+
         def report_message(message):
-            self._manager.reactor.fire("report-job", message)
+            if self.whitelist_patterns:
+                name = message["name"]
+                if not [name for p in self.whitelist_patterns if p.match(name)]:
+                    return
+
+            messages.append(message)
 
         # Set domain and message event handler
         old_domain = gettext.textdomain()
@@ -106,6 +113,23 @@
         for directory in self.directories:
             self._manager.reactor.fire("message-directory", directory)
 
+        # Apply whitelist ordering
+        def cmp_function(a, b):
+            a_name = a["name"]
+            b_name = b["name"]
+            for pattern in self.whitelist_patterns:
+                if pattern.match(a_name):
+                    a_index = self.whitelist_patterns.index(pattern)
+                elif pattern.match(b_name):
+                    b_index = self.whitelist_patterns.index(pattern)
+
+            return cmp(a_index, b_index)
+
+        if self.whitelist_patterns:
+            messages = sorted(messages, cmp=cmp_function)
+        for message in messages:
+            self._manager.reactor.fire("report-job", message)
+
         # Unset domain and event handler
         self._manager.reactor.cancel_call(event_id)
         gettext.textdomain(old_domain)