launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-repository-type-cleanup into lp:launchpad.

Commit message:
Remove GitRepository.repository_type compatibility property and population garbo job.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-repository-type-cleanup/+merge/310709
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-repository-type-cleanup into lp:launchpad.

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2016-10-14 15:08:58 +0000
+++ lib/lp/code/model/gitrepository.py	2016-11-12 21:07:38 +0000
@@ -243,21 +243,8 @@
     date_last_modified = DateTime(
         name='date_last_modified', tzinfo=pytz.UTC, allow_none=False)
 
-    _repository_type = EnumCol(
-        dbName='repository_type', enum=GitRepositoryType, notNull=False)
-
-    @property
-    def repository_type(self):
-        # XXX cjwatson 2016-10-03: Remove once this column has been
-        # backfilled.
-        if self._repository_type is None:
-            return GitRepositoryType.HOSTED
-        else:
-            return self._repository_type
-
-    @repository_type.setter
-    def repository_type(self, value):
-        self._repository_type = value
+    repository_type = EnumCol(
+        dbName='repository_type', enum=GitRepositoryType, notNull=True)
 
     registrant_id = Int(name='registrant', allow_none=False)
     registrant = Reference(registrant_id, 'Person.id')

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2016-10-05 08:57:25 +0000
+++ lib/lp/scripts/garbo.py	2016-11-12 21:07:38 +0000
@@ -41,7 +41,6 @@
     Or,
     Row,
     SQL,
-    Update,
     )
 from storm.info import ClassAlias
 from storm.store import EmptyResultSet
@@ -59,7 +58,6 @@
     BugWatchScheduler,
     MAX_SAMPLE_SIZE,
     )
-from lp.code.enums import GitRepositoryType
 from lp.code.interfaces.revision import IRevisionSet
 from lp.code.model.codeimportevent import CodeImportEvent
 from lp.code.model.codeimportresult import CodeImportResult
@@ -67,7 +65,6 @@
     Diff,
     PreviewDiff,
     )
-from lp.code.model.gitrepository import GitRepository
 from lp.code.model.revision import (
     RevisionAuthor,
     RevisionCache,
@@ -1606,33 +1603,6 @@
         transaction.commit()
 
 
-class GitRepositoryTypePopulator(TunableLoop):
-    """Populates GitRepository.repository_type with HOSTED."""
-
-    maximum_chunk_size = 5000
-
-    def __init__(self, log, abort_time=None):
-        super(GitRepositoryTypePopulator, self).__init__(log, abort_time)
-        self.start_at = 1
-        self.store = IMasterStore(GitRepository)
-
-    def findRepositories(self):
-        return self.store.find(
-            GitRepository,
-            GitRepository.id >= self.start_at,
-            GitRepository._repository_type == None).order_by(GitRepository.id)
-
-    def isDone(self):
-        return self.findRepositories().is_empty()
-
-    def __call__(self, chunk_size):
-        ids = [repository.id for repository in self.findRepositories()]
-        self.store.execute(Update(
-            {GitRepository._repository_type: GitRepositoryType.HOSTED.value},
-            where=GitRepository.id.is_in(ids), table=GitRepository))
-        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.
@@ -1913,7 +1883,6 @@
         CodeImportResultPruner,
         DiffPruner,
         GitJobPruner,
-        GitRepositoryTypePopulator,
         HWSubmissionEmailLinker,
         LiveFSFilePruner,
         LoginTokenPruner,

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2016-10-03 16:34:20 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2016-11-12 21:07:38 +0000
@@ -15,7 +15,6 @@
 from StringIO import StringIO
 import time
 
-from psycopg2 import IntegrityError
 from pytz import UTC
 from storm.exceptions import LostObjectError
 from storm.expr import (
@@ -49,10 +48,7 @@
     BranchFormat,
     RepositoryFormat,
     )
-from lp.code.enums import (
-    CodeImportResultStatus,
-    GitRepositoryType,
-    )
+from lp.code.enums import CodeImportResultStatus
 from lp.code.interfaces.codeimportevent import ICodeImportEventSet
 from lp.code.interfaces.gitrepository import IGitRepositorySet
 from lp.code.model.branchjob import (
@@ -1557,44 +1553,6 @@
         # Snaps with more than one possible store series are untouched.
         self.assertIsNone(snaps[5].store_series)
 
-    def test_GitRepositoryTypePopulator(self):
-        switch_dbuser('testadmin')
-        old_repositories = [self.factory.makeGitRepository() for _ in range(2)]
-        for repository in old_repositories:
-            removeSecurityProxy(repository)._repository_type = None
-        try:
-            Store.of(old_repositories[0]).flush()
-        except IntegrityError:
-            # Now enforced by DB NOT NULL constraint; backfilling is no
-            # longer necessary.
-            return
-        hosted_repositories = [
-            self.factory.makeGitRepository(
-                repository_type=GitRepositoryType.HOSTED)
-            for _ in range(2)]
-        imported_repositories = [
-            self.factory.makeGitRepository(
-                repository_type=GitRepositoryType.IMPORTED)
-            for _ in range(2)]
-        transaction.commit()
-
-        self.runDaily()
-
-        # Old repositories are backfilled.
-        for repository in old_repositories:
-            self.assertEqual(
-                GitRepositoryType.HOSTED,
-                removeSecurityProxy(repository)._repository_type)
-        # Other repositories are left alone.
-        for repository in hosted_repositories:
-            self.assertEqual(
-                GitRepositoryType.HOSTED,
-                removeSecurityProxy(repository)._repository_type)
-        for repository in imported_repositories:
-            self.assertEqual(
-                GitRepositoryType.IMPORTED,
-                removeSecurityProxy(repository)._repository_type)
-
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer