launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/lpsprc-cleanup into lp:launchpad.

Commit message:
Rip out direct calculation code from Person:+*-packages. They now always work from LatestPersonSourcePackageReleaseCache.

Requested reviews:
  William Grant (wgrant): code

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/lpsprc-cleanup/+merge/134831

LatestPersonSourcePackageReleaseCache is populated and functional and enabled on production, so we can now remove the legacy code.
-- 
https://code.launchpad.net/~wgrant/launchpad/lpsprc-cleanup/+merge/134831
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/registry/browser/tests/test_person.py'
--- lib/lp/registry/browser/tests/test_person.py	2012-11-13 13:07:59 +0000
+++ lib/lp/registry/browser/tests/test_person.py	2012-11-19 06:03:19 +0000
@@ -39,7 +39,6 @@
 from lp.registry.model.milestone import milestone_sort_key
 from lp.scripts.garbo import PopulateLatestPersonSourcePackageReleaseCache
 from lp.services.config import config
-from lp.services.features.testing import FeatureFixture
 from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
 from lp.services.log.logger import FakeLogger
@@ -913,15 +912,6 @@
         self.assertEqual(self.view.max_results_to_display, count)
 
 
-class TestFastPersonRelatedPackagesView(TestPersonRelatedPackagesView):
-    # Re-run TestPersonRelatedPackagesView with feature flag on.
-
-    def setUp(self):
-        super(TestFastPersonRelatedPackagesView, self).setUp()
-        self.useFixture(FeatureFixture({
-            'registry.fast_related_software.enabled': 'true'}))
-
-
 class TestPersonMaintainedPackagesView(TestCaseWithFactory):
     """Test the maintained packages view."""
 

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2012-11-16 20:30:12 +0000
+++ lib/lp/registry/model/person.py	2012-11-19 06:03:19 +0000
@@ -2876,9 +2876,6 @@
         """Are there sourcepackagereleases (SPRs) related to this person.
         See `_releasesQueryFilter` for details on the criteria used.
         """
-        if not getFeatureFlag('registry.fast_related_software.enabled'):
-            return self._legacy_hasReleasesQuery(uploader_only, ppa_only)
-
         clauses = self._releasesQueryFilter(uploader_only, ppa_only)
         rs = Store.of(self).using(LatestPersonSourcePackageReleaseCache).find(
             LatestPersonSourcePackageReleaseCache.publication_id, *clauses)
@@ -2888,9 +2885,6 @@
         """Return the sourcepackagereleases records related to this person.
         See `_releasesQueryFilter` for details on the criteria used."""
 
-        if not getFeatureFlag('registry.fast_related_software.enabled'):
-            return self._legacy_latestReleasesQuery(uploader_only, ppa_only)
-
         clauses = self._releasesQueryFilter(uploader_only, ppa_only)
         rs = Store.of(self).find(
             LatestPersonSourcePackageReleaseCache, *clauses).order_by(
@@ -2908,96 +2902,6 @@
 
         return DecoratedResultSet(rs, pre_iter_hook=load_related_objects)
 
-    def _legacy_releasesQueryFilter(self, uploader_only=False, ppa_only=False):
-        """Return the filter used to find sourcepackagereleases (SPRs)
-        related to this person.
-
-        :param uploader_only: controls if we are interested in SPRs where
-            the person in question is only the uploader (creator) and not the
-            maintainer (debian-syncs) if the `ppa_only` parameter is also
-            False, or, if the flag is False, it returns all SPR maintained
-            by this person.
-
-        :param ppa_only: controls if we are interested only in source
-            package releases targeted to any PPAs or, if False, sources
-            targeted to primary archives.
-
-        Active 'ppa_only' flag is usually associated with active
-        'uploader_only' because there shouldn't be any sense of maintainership
-        for packages uploaded to PPAs by someone else than the user himself.
-        """
-        clauses = [SourcePackageRelease.upload_archive == Archive.id]
-
-        if uploader_only:
-            clauses.append(SourcePackageRelease.creator == self)
-
-        if ppa_only:
-            # Source maintainer is irrelevant for PPA uploads.
-            pass
-        elif uploader_only:
-            clauses.append(SourcePackageRelease.maintainer != self)
-        else:
-            clauses.append(SourcePackageRelease.maintainer == self)
-
-        if ppa_only:
-            clauses.append(Archive.purpose == ArchivePurpose.PPA)
-        else:
-            clauses.append(Archive.purpose != ArchivePurpose.PPA)
-
-        return clauses
-
-    def _legacy_hasReleasesQuery(self, uploader_only=False, ppa_only=False):
-        """Are there sourcepackagereleases (SPRs) related to this person.
-        See `_legacy_releasesQueryFilter` for details on the criteria used.
-        """
-        clauses = self._legacy_releasesQueryFilter(uploader_only, ppa_only)
-        spph = ClassAlias(SourcePackagePublishingHistory, "spph")
-        tables = (
-            SourcePackageRelease,
-            Join(
-                spph, spph.sourcepackagereleaseID == SourcePackageRelease.id),
-            Join(Archive, Archive.id == spph.archiveID))
-        rs = Store.of(self).using(*tables).find(
-            SourcePackageRelease.id, clauses)
-        return not rs.is_empty()
-
-    def _legacy_latestReleasesQuery(self, uploader_only=False, ppa_only=False):
-        """Return the sourcepackagereleases (SPRs) related to this person.
-        See `_legacy_releasesQueryFilter` for details on the criteria used."""
-        clauses = self._legacy_releasesQueryFilter(uploader_only, ppa_only)
-        spph = ClassAlias(SourcePackagePublishingHistory, "spph")
-        rs = Store.of(self).find(
-            SourcePackageRelease,
-            SourcePackageRelease.id.is_in(
-                Select(
-                    SourcePackageRelease.id,
-                    tables=[
-                        SourcePackageRelease,
-                        Join(
-                            spph,
-                            spph.sourcepackagereleaseID ==
-                            SourcePackageRelease.id),
-                        Join(Archive, Archive.id == spph.archiveID)],
-                    where=And(*clauses),
-                    order_by=[SourcePackageRelease.upload_distroseriesID,
-                              SourcePackageRelease.sourcepackagenameID,
-                              SourcePackageRelease.upload_archiveID,
-                              Desc(SourcePackageRelease.dateuploaded)],
-                    distinct=(
-                        SourcePackageRelease.upload_distroseriesID,
-                        SourcePackageRelease.sourcepackagenameID,
-                        SourcePackageRelease.upload_archiveID)))
-        ).order_by(
-            Desc(SourcePackageRelease.dateuploaded), SourcePackageRelease.id)
-
-        def load_related_objects(rows):
-            list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
-                set(map(attrgetter("maintainerID"), rows))))
-            bulk.load_related(SourcePackageName, rows, ['sourcepackagenameID'])
-            bulk.load_related(Archive, rows, ['upload_archiveID'])
-
-        return DecoratedResultSet(rs, pre_iter_hook=load_related_objects)
-
     def hasSynchronisedPublishings(self):
         """See `IPerson`."""
         spph = ClassAlias(SourcePackagePublishingHistory, "spph")

=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2012-11-14 16:30:37 +0000
+++ lib/lp/services/features/flags.py	2012-11-19 06:03:19 +0000
@@ -238,14 +238,6 @@
      'Traversal to private projects requires special access.',
      'Override traveral checks.',
      'https://dev.launchpad.net/LEP/PrivateProjects'),
-    ('registry.fast_related_software.enabled',
-     'boolean',
-     ('If true, use the new reporting cache to load a person\'s related '
-      'software information.'),
-     '',
-     '',
-     ''),
-
     ])
 
 # The set of all flag names that are documented.