launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:git-repo-async-provacy-garbo into launchpad:master with ~pappacena/launchpad:git-repo-async-privacy as a prerequisite.

Commit message:
Adding garbo to update GitRepository's status if corresponding information type change job fails

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/392809
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:git-repo-async-provacy-garbo into launchpad:master.

diff --git a/lib/lp/scripts/garbo.py b/lib/lp/scripts/garbo.py
index 6e0feac..ea248a4 100644
--- a/lib/lp/scripts/garbo.py
+++ b/lib/lp/scripts/garbo.py
@@ -38,6 +38,7 @@ from storm.expr import (
     Cast,
     In,
     Join,
+    LeftJoin,
     Max,
     Min,
     Or,
@@ -68,6 +69,10 @@ from lp.code.model.diff import (
     Diff,
     PreviewDiff,
     )
+from lp.code.model.gitjob import (
+    GitJob,
+    GitJobType,
+    )
 from lp.code.model.gitrepository import GitRepository
 from lp.code.model.revision import (
     RevisionAuthor,
@@ -1554,6 +1559,54 @@ class GitRepositoryPruner(TunableLoop):
         transaction.commit()
 
 
+class GitRepositoryBrokenInfoTypeTransition(TunableLoop):
+    """Put back to "AVAILABLE" repositories that are pending information
+    type changes, but we don't have any git job that will actually do that
+    in the upcoming future.
+    """
+
+    maximum_chunk_size = 500
+
+    def __init__(self, log, abort_time=None):
+        super(GitRepositoryBrokenInfoTypeTransition, self).__init__(
+            log, abort_time)
+        self.store = IMasterStore(GitRepository)
+
+    def findRepositories(self):
+        pending_change = (
+            GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION)
+        job_type = GitJobType.REPOSITORY_TRANSITION_TO_INFO_TYPE
+        job_pending_statuses = (JobStatus.WAITING, JobStatus.RUNNING)
+        # Get git repositories left-joining with
+        # REPOSITORY_TRANSITION_TO_INFO_TYPE GitJobs waiting to be run (or
+        # already running).
+        join = [
+            GitRepository,
+            LeftJoin(
+                GitJob,
+                And(GitJob.repository_id == GitRepository.id,
+                    GitJob.job_type == job_type)),
+            LeftJoin(
+                Job,
+                And(GitJob.job_id == Job.id,
+                    Job._status.is_in(job_pending_statuses)))]
+        # We get only the repositories pending change without associated job.
+        result_set = self.store.using(*join).find(
+            GitRepository,
+            GitRepository.status == pending_change,
+            Job._status == None)
+        return result_set.order_by(GitRepository.date_created)
+
+    def isDone(self):
+        return self.findRepositories().is_empty()
+
+    def __call__(self, chunk_size):
+        repositories = self.findRepositories()[:chunk_size]
+        for repository in repositories:
+            repository.status = GitRepositoryStatus.AVAILABLE
+        transaction.commit()
+
+
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
     script_name = None  # Script name for locking and database user. Override.
@@ -1806,6 +1859,7 @@ class HourlyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
         BugHeatUpdater,
         DuplicateSessionPruner,
         GitRepositoryPruner,
+        GitRepositoryBrokenInfoTypeTransition,
         RevisionCachePruner,
         UnusedSessionPruner,
         ]
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index 2fbc80d..ae3b0f1 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -1126,6 +1126,66 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
             {old_available, recent_available, recent_creating},
             set(remaining_repos))
 
+    def test_GitRepositoryBrokenInfoTypeTransition_changes_status(self):
+        self.useFixture(FeatureFixture({OCI_RECIPE_ALLOW_CREATE: 'on'}))
+        # Shortcuts.
+        available = GitRepositoryStatus.AVAILABLE
+        pending_transition = (
+            GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION)
+
+        switch_dbuser('testadmin')
+        store = IMasterStore(GitRepository)
+        created_repos = [self.factory.makeGitRepository() for i in range(5)]
+
+        pending_without_job_repos = []
+        for i in range(2):
+            repo = self.factory.makeGitRepository()
+            removeSecurityProxy(repo)._status = pending_transition
+
+        pending_with_failed_jobs = []
+        for i in range(3):
+            repo = self.factory.makeGitRepository()
+            # For some repos, create the job but force them to fail
+            job = repo.transitionToInformationType(
+                InformationType.PRIVATESECURITY, repo.owner)
+            job.start()
+            job.fail()
+            pending_with_failed_jobs.append(repo)
+
+        pending_with_started_job_repos = []
+        for i in range(2):
+            repo = self.factory.makeGitRepository()
+            job = repo.transitionToInformationType(
+                InformationType.PRIVATESECURITY, repo.owner)
+            job.start()
+            pending_with_started_job_repos.append(repo)
+
+        pending_with_waiting_jobs = []
+        for i in range(3):
+            repo = self.factory.makeGitRepository()
+            repo.transitionToInformationType(
+                InformationType.PRIVATESECURITY, repo.owner)
+            pending_with_started_job_repos.append(repo)
+
+        self.assertEqual(15, store.find(GitRepository).count())
+
+        self.runHourly(maximum_chunk_size=2)
+
+        switch_dbuser('testadmin')
+        self.assertEqual(15, store.find(GitRepository).count())
+        self.assertTrue(
+            all(i.status == available for i in created_repos))
+        self.assertTrue(
+            all(i.status == available for i in pending_without_job_repos))
+        self.assertTrue(
+            all(i.status == available for i in pending_with_failed_jobs))
+        self.assertTrue(
+            all(i.status == pending_transition
+                for i in pending_with_started_job_repos))
+        self.assertTrue(
+            all(i.status == pending_transition
+                for i in pending_with_waiting_jobs))
+
     def test_WebhookJobPruner(self):
         # Garbo should remove jobs completed over 30 days ago.
         switch_dbuser('testadmin')