Merge ~pappacena/launchpad:git-repo-async-provacy-garbo into launchpad:master

Proposed by Thiago F. Pappacena
Status: Needs review
Proposed branch: ~pappacena/launchpad:git-repo-async-provacy-garbo
Merge into: launchpad:master
Prerequisite: ~pappacena/launchpad:git-repo-async-privacy
Diff against target: 157 lines (+114/-0)
2 files modified
lib/lp/scripts/garbo.py (+54/-0)
lib/lp/scripts/tests/test_garbo.py (+60/-0)
Reviewer Review Type Date Requested Status
Ioana Lasc Approve
Review via email: mp+392809@code.launchpad.net

Commit message

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

To post a comment you must log in.
Revision history for this message
Ioana Lasc (ilasc) wrote :

LGTM

review: Approve

Unmerged commits

6118345... by Thiago F. Pappacena on 2020-10-26

Improving tests for more scenarios

20ea95f... by Thiago F. Pappacena on 2020-10-26

Adding garbo to keep GitRepo status in line with info type change job

4bd7dcc... by Thiago F. Pappacena on 2020-10-26

Fixing configuration of info type transition job

76e0272... by Thiago F. Pappacena on 2020-10-23

Blocking at the UI information type change while it is already changing

caf2673... by Thiago F. Pappacena on 2020-10-23

Adding tests for async GitRepo.transitionToInformationType

3a5e053... by Thiago F. Pappacena on 2020-10-23

Changing GitRepo.transitionInformationType method to be async

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/scripts/garbo.py b/lib/lp/scripts/garbo.py
2index 6e0feac..ea248a4 100644
3--- a/lib/lp/scripts/garbo.py
4+++ b/lib/lp/scripts/garbo.py
5@@ -38,6 +38,7 @@ from storm.expr import (
6 Cast,
7 In,
8 Join,
9+ LeftJoin,
10 Max,
11 Min,
12 Or,
13@@ -68,6 +69,10 @@ from lp.code.model.diff import (
14 Diff,
15 PreviewDiff,
16 )
17+from lp.code.model.gitjob import (
18+ GitJob,
19+ GitJobType,
20+ )
21 from lp.code.model.gitrepository import GitRepository
22 from lp.code.model.revision import (
23 RevisionAuthor,
24@@ -1554,6 +1559,54 @@ class GitRepositoryPruner(TunableLoop):
25 transaction.commit()
26
27
28+class GitRepositoryBrokenInfoTypeTransition(TunableLoop):
29+ """Put back to "AVAILABLE" repositories that are pending information
30+ type changes, but we don't have any git job that will actually do that
31+ in the upcoming future.
32+ """
33+
34+ maximum_chunk_size = 500
35+
36+ def __init__(self, log, abort_time=None):
37+ super(GitRepositoryBrokenInfoTypeTransition, self).__init__(
38+ log, abort_time)
39+ self.store = IMasterStore(GitRepository)
40+
41+ def findRepositories(self):
42+ pending_change = (
43+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION)
44+ job_type = GitJobType.REPOSITORY_TRANSITION_TO_INFO_TYPE
45+ job_pending_statuses = (JobStatus.WAITING, JobStatus.RUNNING)
46+ # Get git repositories left-joining with
47+ # REPOSITORY_TRANSITION_TO_INFO_TYPE GitJobs waiting to be run (or
48+ # already running).
49+ join = [
50+ GitRepository,
51+ LeftJoin(
52+ GitJob,
53+ And(GitJob.repository_id == GitRepository.id,
54+ GitJob.job_type == job_type)),
55+ LeftJoin(
56+ Job,
57+ And(GitJob.job_id == Job.id,
58+ Job._status.is_in(job_pending_statuses)))]
59+ # We get only the repositories pending change without associated job.
60+ result_set = self.store.using(*join).find(
61+ GitRepository,
62+ GitRepository.status == pending_change,
63+ Job._status == None)
64+ return result_set.order_by(GitRepository.date_created)
65+
66+ def isDone(self):
67+ return self.findRepositories().is_empty()
68+
69+ def __call__(self, chunk_size):
70+ repositories = self.findRepositories()[:chunk_size]
71+ for repository in repositories:
72+ repository.status = GitRepositoryStatus.AVAILABLE
73+ transaction.commit()
74+
75+
76 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
77 """Abstract base class to run a collection of TunableLoops."""
78 script_name = None # Script name for locking and database user. Override.
79@@ -1806,6 +1859,7 @@ class HourlyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
80 BugHeatUpdater,
81 DuplicateSessionPruner,
82 GitRepositoryPruner,
83+ GitRepositoryBrokenInfoTypeTransition,
84 RevisionCachePruner,
85 UnusedSessionPruner,
86 ]
87diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
88index 2fbc80d..ae3b0f1 100644
89--- a/lib/lp/scripts/tests/test_garbo.py
90+++ b/lib/lp/scripts/tests/test_garbo.py
91@@ -1126,6 +1126,66 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
92 {old_available, recent_available, recent_creating},
93 set(remaining_repos))
94
95+ def test_GitRepositoryBrokenInfoTypeTransition_changes_status(self):
96+ self.useFixture(FeatureFixture({OCI_RECIPE_ALLOW_CREATE: 'on'}))
97+ # Shortcuts.
98+ available = GitRepositoryStatus.AVAILABLE
99+ pending_transition = (
100+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION)
101+
102+ switch_dbuser('testadmin')
103+ store = IMasterStore(GitRepository)
104+ created_repos = [self.factory.makeGitRepository() for i in range(5)]
105+
106+ pending_without_job_repos = []
107+ for i in range(2):
108+ repo = self.factory.makeGitRepository()
109+ removeSecurityProxy(repo)._status = pending_transition
110+
111+ pending_with_failed_jobs = []
112+ for i in range(3):
113+ repo = self.factory.makeGitRepository()
114+ # For some repos, create the job but force them to fail
115+ job = repo.transitionToInformationType(
116+ InformationType.PRIVATESECURITY, repo.owner)
117+ job.start()
118+ job.fail()
119+ pending_with_failed_jobs.append(repo)
120+
121+ pending_with_started_job_repos = []
122+ for i in range(2):
123+ repo = self.factory.makeGitRepository()
124+ job = repo.transitionToInformationType(
125+ InformationType.PRIVATESECURITY, repo.owner)
126+ job.start()
127+ pending_with_started_job_repos.append(repo)
128+
129+ pending_with_waiting_jobs = []
130+ for i in range(3):
131+ repo = self.factory.makeGitRepository()
132+ repo.transitionToInformationType(
133+ InformationType.PRIVATESECURITY, repo.owner)
134+ pending_with_started_job_repos.append(repo)
135+
136+ self.assertEqual(15, store.find(GitRepository).count())
137+
138+ self.runHourly(maximum_chunk_size=2)
139+
140+ switch_dbuser('testadmin')
141+ self.assertEqual(15, store.find(GitRepository).count())
142+ self.assertTrue(
143+ all(i.status == available for i in created_repos))
144+ self.assertTrue(
145+ all(i.status == available for i in pending_without_job_repos))
146+ self.assertTrue(
147+ all(i.status == available for i in pending_with_failed_jobs))
148+ self.assertTrue(
149+ all(i.status == pending_transition
150+ for i in pending_with_started_job_repos))
151+ self.assertTrue(
152+ all(i.status == pending_transition
153+ for i in pending_with_waiting_jobs))
154+
155 def test_WebhookJobPruner(self):
156 # Garbo should remove jobs completed over 30 days ago.
157 switch_dbuser('testadmin')