Shrink the size of the argument to SCMProvider.check_for_update

We don't need the whole job object, just its version is sufficient.

This is an ugly fix for a recursion overflow in the memoize
implementation where we pickle the argument.

A better fix would be to fix memoize.
Or revisit the need to have a linked list of every single job
for a library.

Fixes #403

Author: tomrittervg

components/scmprovider.py

@@ -115,7 +115,7 @@ def _ensure_checkout(self, repo_url, specific_revision=None):
 
     @logEntryExit
     @Memoize
-    def check_for_update(self, library, task, new_version, most_recent_job):
+    def check_for_update(self, library, task, new_version, most_recent_job_version):
         # This function uses two tricky variable names:
         #  all_upstream_commits - This means the commits that have occured upstream, on the branch we care about,
         #                         between the library's current revision and the tip of the branch.
@@ -159,8 +159,8 @@ def check_for_update(self, library, task, new_version, most_recent_job):
             #        in m-c we update the library to B (or maybe even a new rev C with no job)
             #        Resulting in the most recent job (which was for B) occured _before_ the library's current revision
             # We can only do this if we have a most recent job, if we don't we're processing this library for the first time
-            if most_recent_job:
-                most_recent_job_newer_than_library_rev = most_recent_job.version in [c.revision for c in all_new_upstream_commits]
+            if most_recent_job_version:
+                most_recent_job_newer_than_library_rev = most_recent_job_version in [c.revision for c in all_new_upstream_commits]
                 if most_recent_job_newer_than_library_rev:
                     self.logger.log("The most recent job we have run is for a revision still upstream and not in the mozilla repo.", level=LogLevel.Debug)
                 else:
@@ -172,11 +172,11 @@ def check_for_update(self, library, task, new_version, most_recent_job):
             unseen_new_upstream_commits = []
             if most_recent_job_newer_than_library_rev:
                 # Step 5: Get the list of commits between the revision for the most recent job
-                # and new_version. (We previously confirmed that most_recent_job.version is in the sequence
+                # and new_version. (We previously confirmed that most_recent_job_version is in the sequence
                 # of commits from common_ancestor..new_version)
-                unseen_new_upstream_commits = self._commits_between(most_recent_job.version, new_version)
+                unseen_new_upstream_commits = self._commits_between(most_recent_job_version, new_version)
                 if len(unseen_new_upstream_commits) == 0:
-                    self.logger.log("Already processed revision %s in bug %s" % (most_recent_job.version, most_recent_job.bugzilla_id), level=LogLevel.Info)
+                    self.logger.log("Already processed revision %s" % (most_recent_job_version), level=LogLevel.Info)
                     return all_new_upstream_commits, []
 
                 # Step 6: Ensure that the unseen list of a subset of the 'all-new' list

tasktypes/base.py

@@ -65,7 +65,7 @@ def frequency_type():
                 return False
 
         if commit_count > 0:
-            all_upstream_commits, unseen_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, most_recent_job)
+            all_upstream_commits, unseen_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, most_recent_job.version if most_recent_job else None)
             commits_since_in_tree = len(all_upstream_commits)
             commits_since_new_job = len(unseen_upstream_commits)
 

tasktypes/commitalert.py

@@ -33,7 +33,7 @@ def process_task(self, library, task):
         my_ff_version = self.config['General']['ff-version']
 
         all_library_jobs = self.dbProvider.get_all_jobs_for_library(library, JOBTYPE.COMMITALERT)
-        all_upstream_commits, unseen_upstream_commits = self.scmProvider.check_for_update(library, task, task.branch or "HEAD", all_library_jobs[0] if all_library_jobs else None)
+        all_upstream_commits, unseen_upstream_commits = self.scmProvider.check_for_update(library, task, task.branch or "HEAD", all_library_jobs[0].version if all_library_jobs else None)
         self.logger.log("We found %s previous jobs, %s upstream commits, and %s unseen upstream commits." % (
             len(all_library_jobs), len(all_upstream_commits), len(unseen_upstream_commits)), level=LogLevel.Info)
 

tasktypes/vendoring.py

@@ -130,7 +130,7 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo
         self.logger.set_context(library.name, created_job.id)
 
         # File the bug ------------------------
-        all_upstream_commits, unseen_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, most_recent_job)
+        all_upstream_commits, unseen_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, most_recent_job.version if most_recent_job else None)
         commit_stats = self.mercurialProvider.diff_stats()
         commit_details = self.scmProvider.build_bug_description(all_upstream_commits, 65534 - len(commit_stats) - 220) if library.should_show_commit_details else ""
 

tests/frequency.py

@@ -77,15 +77,15 @@ def testFrequency(self):
         self.assertFalse(bt._should_process_new_job(library, task))
 
         task.frequency = '1 week, 3 commits'
-        bt.dbProvider.get_all_jobs_for_library = lambda a, b: [Struct(**{"created": datetime.now() - timedelta(weeks=1, hours=1)})]
+        bt.dbProvider.get_all_jobs_for_library = lambda a, b: [Struct(**{"created": datetime.now() - timedelta(weeks=1, hours=1), "version": "whatever"})]
         self.assertFalse(bt._should_process_new_job(library, task))
 
         task.frequency = '2 weeks, 2 commits'
-        bt.dbProvider.get_all_jobs_for_library = lambda a, b: [Struct(**{"created": datetime.now() - timedelta(weeks=1, hours=1)})]
+        bt.dbProvider.get_all_jobs_for_library = lambda a, b: [Struct(**{"created": datetime.now() - timedelta(weeks=1, hours=1), "version": "whatever"})]
         self.assertFalse(bt._should_process_new_job(library, task))
 
         task.frequency = '1 week, 2 commits'
-        bt.dbProvider.get_all_jobs_for_library = lambda a, b: [Struct(**{"created": datetime.now() - timedelta(weeks=1, hours=1)})]
+        bt.dbProvider.get_all_jobs_for_library = lambda a, b: [Struct(**{"created": datetime.now() - timedelta(weeks=1, hours=1), "version": "whatever"})]
         self.assertTrue(bt._should_process_new_job(library, task))
 
 

tests/scm.py

@@ -92,7 +92,7 @@ def testCheckForUpdates(self):
         new_version = COMMITS_MAIN[0]
         library.revision = COMMITS_MAIN[-1]
         ignoreme = Struct(**{'version': COMMITS_MAIN[3]})
-        all_new_upstream_commits, unseen_new_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, ignoreme)
+        all_new_upstream_commits, unseen_new_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, ignoreme.version)
         self.assertEqual(len(all_new_upstream_commits), len(COMMITS_MAIN) - 1)
         for i in range(len(COMMITS_MAIN) - 1):
             self.assertEqual(all_new_upstream_commits[i].revision, COMMITS_MAIN_R[i + 1])
@@ -115,7 +115,7 @@ def testCheckForUpdatesOnTagSkipOne(self):
         new_version = 'v0.0.2'
         library.revision = 'v0.0.1'
         ignoreme = Struct(**{'version': COMMITS_MAIN[-4]})
-        all_new_upstream_commits, unseen_new_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, ignoreme)
+        all_new_upstream_commits, unseen_new_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, ignoreme.version)
         self.assertEqual(len(all_new_upstream_commits), 2)
         self.assertEqual(len(unseen_new_upstream_commits), 1)