launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14430
[Merge] lp:~wgrant/launchpad/batch-publishinghistory into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/batch-publishinghistory into lp:launchpad.
Commit message:
Batch DistributionSourcePackage:+publishinghistory
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #739066 in Launchpad itself: "DistributionSourcePackage:+publishinghistory timeouts"
https://bugs.launchpad.net/launchpad/+bug/739066
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/batch-publishinghistory/+merge/135330
This branch batches DistributionSourcePackage:+publishinghistory, as some packages otherwise try to render several hundred publications and time out. Due to a shared template DistributionSourcePackageRelease:+publishinghistory is also now batched, although that's far less important due to the more restricted scope.
I cleaned up various methods and properties relating to publishing histories on the non-DB-backed objects, saving a fair bit of LoC.
--
https://code.launchpad.net/~wgrant/launchpad/batch-publishinghistory/+merge/135330
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/batch-publishinghistory into lp:launchpad.
=== modified file 'lib/lp/registry/browser/distributionsourcepackage.py'
--- lib/lp/registry/browser/distributionsourcepackage.py 2012-09-12 04:29:55 +0000
+++ lib/lp/registry/browser/distributionsourcepackage.py 2012-11-21 08:32:19 +0000
@@ -14,6 +14,7 @@
'DistributionSourcePackageOverviewMenu',
'DistributionSourcePackagePublishingHistoryView',
'DistributionSourcePackageView',
+ 'PublishingHistoryViewMixin',
]
import itertools
@@ -55,6 +56,7 @@
from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
from lp.registry.browser import add_subscribe_link
from lp.registry.browser.pillar import PillarBugsMenu
+from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.registry.interfaces.distributionsourcepackage import (
IDistributionSourcePackage,
)
@@ -69,6 +71,7 @@
redirection,
StandardLaunchpadFacets,
)
+from lp.services.webapp.batching import BatchNavigator
from lp.services.webapp.breadcrumb import Breadcrumb
from lp.services.webapp.interfaces import IBreadcrumb
from lp.services.webapp.menu import (
@@ -581,15 +584,12 @@
return 'Change log for %s' % self.context.title
-class DistributionSourcePackagePublishingHistoryView(LaunchpadView):
- """View for presenting `DistributionSourcePackage` publishing history."""
-
- page_title = 'Publishing history'
-
- def initialize(self):
- """Preload relevant `IPerson` objects."""
+class PublishingHistoryViewMixin:
+ """Mixin for presenting batches of `SourcePackagePublishingHistory`s."""
+
+ def _preload_people(self, pubs):
ids = set()
- for spph in self.context.publishing_history:
+ for spph in pubs:
ids.update((spph.removed_byID, spph.creatorID, spph.sponsorID))
ids.discard(None)
if ids:
@@ -597,6 +597,23 @@
ids, need_validity=True))
@property
+ def batchnav(self):
+ # No point using StormRangeFactory right now, as the sorted
+ # lookup can't be fully indexed (it spans multiple archives).
+ return BatchNavigator(
+ DecoratedResultSet(
+ self.context.publishing_history,
+ pre_iter_hook=self._preload_people),
+ self.request)
+
+
+class DistributionSourcePackagePublishingHistoryView(
+ LaunchpadView, PublishingHistoryViewMixin):
+ """View for presenting `DistributionSourcePackage` publishing history."""
+
+ page_title = 'Publishing history'
+
+ @property
def label(self):
return 'Publishing history of %s' % self.context.title
=== modified file 'lib/lp/registry/browser/tests/test_distributionsourcepackage.py'
--- lib/lp/registry/browser/tests/test_distributionsourcepackage.py 2012-08-09 16:39:41 +0000
+++ lib/lp/registry/browser/tests/test_distributionsourcepackage.py 2012-11-21 08:32:19 +0000
@@ -12,6 +12,8 @@
from lp.app.enums import ServiceUsage
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.services.webapp import canonical_url
+from lp.soyuz.enums import PackagePublishingStatus
from lp.soyuz.interfaces.archive import ArchivePurpose
from lp.testing import (
celebrity_logged_in,
@@ -23,7 +25,10 @@
DatabaseFunctionalLayer,
LaunchpadFunctionalLayer,
)
-from lp.testing.matchers import BrowsesWithQueryLimit
+from lp.testing.matchers import (
+ BrowsesWithQueryLimit,
+ IsConfiguredBatchNavigator,
+ )
from lp.testing.views import (
create_initialized_view,
create_view,
@@ -91,7 +96,7 @@
# This is a lot of extra queries per publication, and should be
# ratcheted down over time; but it at least ensures that we don't
# make matters any worse.
- publishinghistory_browses_under_limit.query_limit += 10
+ publishinghistory_browses_under_limit.query_limit += 7
self.assertThat(dsp, publishinghistory_browses_under_limit)
def test_show_sponsor(self):
@@ -122,6 +127,32 @@
)
self.assertThat(html, record_matches)
+ def test_is_batched(self):
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
+ spn = self.factory.makeSourcePackageName()
+ component = self.factory.makeComponent()
+ section = self.factory.makeSection()
+ dsp = archive.distribution.getSourcePackage(spn)
+ statuses = (
+ [PackagePublishingStatus.PUBLISHED]
+ + ([PackagePublishingStatus.SUPERSEDED] * 5))
+ for status in statuses:
+ self.factory.makeSourcePackagePublishingHistory(
+ archive=archive, sourcepackagename=spn, component=component,
+ distroseries=archive.distribution.currentseries,
+ section_name=section.name, status=status)
+ view = create_initialized_view(dsp, "+publishinghistory")
+ self.assertThat(
+ view.batchnav, IsConfiguredBatchNavigator('result', 'results'))
+
+ base_url = canonical_url(dsp) + '/+publishinghistory'
+ browser = self.getUserBrowser(base_url)
+ self.assertIn("<td>Published</td>", browser.contents)
+ self.assertIn("<td>Superseded</td>", browser.contents)
+ browser.getLink("Next").click()
+ self.assertNotIn("<td>Published</td>", browser.contents)
+ self.assertIn("<td>Superseded</td>", browser.contents)
+
class TestDistributionSourceView(TestCaseWithFactory):
=== modified file 'lib/lp/registry/doc/distroseries.txt'
--- lib/lp/registry/doc/distroseries.txt 2012-09-28 14:35:35 +0000
+++ lib/lp/registry/doc/distroseries.txt 2012-11-21 08:32:19 +0000
@@ -551,17 +551,6 @@
>>> pmount_srcrel.publishing_history.count()
1
-Most recent published history row:
-
- >>> pmount_srcrel.current_published is None
- True
-
- >>> netapplet_srcrel = hoary.getSourcePackage('netapplet').currentrelease
- >>> spph = netapplet_srcrel.current_published
-
- >>> spph.section.name
- u'web'
-
The changesfile attribute contains the package changelog. It is provided as
an ILibraryFileAlias:
@@ -575,6 +564,7 @@
If the package changelog is not available, that attribute is None:
+ >>> netapplet_srcrel = hoary.getSourcePackage('netapplet').currentrelease
>>> netapplet_srcrel.changesfile is None
True
@@ -582,7 +572,7 @@
>>> new_section = getUtility(ISectionSet)['base']
- >>> override = netapplet_srcrel.current_published.changeOverride(
+ >>> override = netapplet_srcrel.publishing_history.first().changeOverride(
... new_section=new_section)
>>> override.section == new_section
@@ -596,10 +586,11 @@
Override information about 'pmount' is pending publication:
- >>> print netapplet_srcrel.current_published.status.name
+ >>> overridden = netapplet_srcrel.publishing_history.first()
+ >>> print overridden.status.name
PENDING
- >>> print netapplet_srcrel.current_published.section.name
+ >>> print overridden.section.name
base
Supersede previous netapplet publication:
=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py 2012-09-25 23:54:26 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py 2012-11-21 08:32:19 +0000
@@ -54,6 +54,7 @@
SourcePackage,
SourcePackageQuestionTargetMixin,
)
+from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.services.database.lpstorm import IStore
from lp.services.database.sqlbase import sqlvalues
from lp.services.propertycache import cachedproperty
@@ -388,26 +389,27 @@
return self._getPublishingHistoryQuery(status)
def _getPublishingHistoryQuery(self, status=None):
- query = """
- DistroSeries.distribution = %s AND
- SourcePackagePublishingHistory.archive IN %s AND
- SourcePackagePublishingHistory.distroseries =
- DistroSeries.id AND
- SourcePackagePublishingHistory.sourcepackagename = %s AND
- SourcePackageRelease.id =
- SourcePackagePublishingHistory.sourcepackagerelease
- """ % sqlvalues(self.distribution,
- self.distribution.all_distro_archive_ids,
- self.sourcepackagename)
+ conditions = [
+ SourcePackagePublishingHistory.archiveID.is_in(
+ self.distribution.all_distro_archive_ids),
+ SourcePackagePublishingHistory.distroseriesID == DistroSeries.id,
+ DistroSeries.distribution == self.distribution,
+ SourcePackagePublishingHistory.sourcepackagename ==
+ self.sourcepackagename,
+ SourcePackageRelease.id ==
+ SourcePackagePublishingHistory.sourcepackagereleaseID,
+ ]
if status is not None:
- query += ("AND SourcePackagePublishingHistory.status = %s"
- % sqlvalues(status))
+ conditions.append(SourcePackagePublishingHistory.status == status)
- return SourcePackagePublishingHistory.select(query,
- clauseTables=['DistroSeries', 'SourcePackageRelease'],
- prejoinClauseTables=['SourcePackageRelease'],
- orderBy='-datecreated')
+ res = IStore(SourcePackagePublishingHistory).find(
+ (SourcePackagePublishingHistory, SourcePackageRelease),
+ *conditions)
+ res.order_by(
+ Desc(SourcePackagePublishingHistory.datecreated),
+ Desc(SourcePackagePublishingHistory.id))
+ return DecoratedResultSet(res, operator.itemgetter(0))
def getReleasesAndPublishingHistory(self):
"""See `IDistributionSourcePackage`."""
=== modified file 'lib/lp/services/database/decoratedresultset.py'
--- lib/lp/services/database/decoratedresultset.py 2012-07-05 17:08:12 +0000
+++ lib/lp/services/database/decoratedresultset.py 2012-11-21 08:32:19 +0000
@@ -75,7 +75,9 @@
return [], []
elif (zope_isinstance(self.result_set, DecoratedResultSet)
and self.return_both):
- assert self.result_set.return_both == self.return_both
+ assert (
+ removeSecurityProxy(self.result_set).return_both
+ == self.return_both)
return zip(*results)
else:
return results, results
=== modified file 'lib/lp/soyuz/browser/distributionsourcepackagerelease.py'
--- lib/lp/soyuz/browser/distributionsourcepackagerelease.py 2012-07-10 08:38:23 +0000
+++ lib/lp/soyuz/browser/distributionsourcepackagerelease.py 2012-11-21 08:32:19 +0000
@@ -15,6 +15,9 @@
from lazr.restful.utils import smartquote
from lp.archivepublisher.debversion import Version
+from lp.registry.browser.distributionsourcepackage import (
+ PublishingHistoryViewMixin,
+ )
from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.propertycache import cachedproperty
from lp.services.webapp import (
@@ -136,7 +139,8 @@
return distroseries_builds
-class DistributionSourcePackageReleasePublishingHistoryView(LaunchpadView):
+class DistributionSourcePackageReleasePublishingHistoryView(
+ LaunchpadView, PublishingHistoryViewMixin):
"""Presenting `DistributionSourcePackageRelease` publishing history."""
usedfor = IDistributionSourcePackageRelease
=== modified file 'lib/lp/soyuz/doc/sourcepackagerelease-build-lookup.txt'
--- lib/lp/soyuz/doc/sourcepackagerelease-build-lookup.txt 2012-09-27 02:53:00 +0000
+++ lib/lp/soyuz/doc/sourcepackagerelease-build-lookup.txt 2012-11-21 08:32:19 +0000
@@ -89,8 +89,8 @@
>>> print breezy_autotest.previous_series.title
The Hoary Hedgehog Release
- >>> hoary_evo_release = hoary_evo_source['1.0']
- >>> copied_pub = hoary_evo_release.current_published.copyTo(
+ >>> evo_pub = hoary.getPublishedSources('evolution', version='1.0')[0]
+ >>> copied_pub = evo_pub.copyTo(
... breezy_autotest, pocket_release, breezy_autotest.main_archive)
>>> breezy_autotest_evo_source = breezy_autotest.getSourcePackage(
@@ -176,8 +176,7 @@
Let's start by copying a PRIMARY archive source to Celso's PPA.
- >>> copy = hoary_evo_source['1.0'].current_published.copyTo(
- ... hoary, pocket_release, cprov.archive)
+ >>> copy = evo_pub.copyTo(hoary, pocket_release, cprov.archive)
No suitable build will be found for it.
=== modified file 'lib/lp/soyuz/doc/soyuz-upload.txt'
--- lib/lp/soyuz/doc/soyuz-upload.txt 2012-06-29 08:40:05 +0000
+++ lib/lp/soyuz/doc/soyuz-upload.txt 2012-11-21 08:32:19 +0000
@@ -501,7 +501,7 @@
>>> breezy_autotest = ubuntutest['breezy-autotest']
>>> etherwake = breezy_autotest.getSourcePackage('etherwake')
>>> etherwake_drspr = etherwake.currentrelease
- >>> override = etherwake_drspr.current_published.changeOverride(
+ >>> override = etherwake_drspr.publishing_history.first().changeOverride(
... new_component=getUtility(IComponentSet)['multiverse'])
Check if we have new pending publishing record as expected
=== modified file 'lib/lp/soyuz/interfaces/distroseriessourcepackagerelease.py'
--- lib/lp/soyuz/interfaces/distroseriessourcepackagerelease.py 2011-12-24 16:54:44 +0000
+++ lib/lp/soyuz/interfaces/distroseriessourcepackagerelease.py 2012-11-21 08:32:19 +0000
@@ -54,9 +54,6 @@
"Return binaries resulted from this sourcepackagerelease and "
"published in this distroseries.")
- current_published = Attribute("is last SourcePackagePublishing record "
- "that is in PUBLISHED status.")
-
version = Attribute("The version of the source package release.")
changesfile = Object(
=== modified file 'lib/lp/soyuz/model/distributionsourcepackagerelease.py'
--- lib/lp/soyuz/model/distributionsourcepackagerelease.py 2012-09-28 06:25:44 +0000
+++ lib/lp/soyuz/model/distributionsourcepackagerelease.py 2012-11-21 08:32:19 +0000
@@ -19,17 +19,12 @@
LeftJoin,
SQL,
)
-from zope.component import getUtility
+from storm.store import Store
from zope.interface import implements
from lp.buildmaster.model.buildfarmjob import BuildFarmJob
from lp.buildmaster.model.packagebuild import PackageBuild
from lp.services.database.decoratedresultset import DecoratedResultSet
-from lp.services.database.interfaces import (
- DEFAULT_FLAVOR,
- IStoreSelector,
- MAIN_STORE,
- )
from lp.services.database.sqlbase import sqlvalues
from lp.soyuz.interfaces.archive import MAIN_ARCHIVE_PURPOSES
from lp.soyuz.interfaces.distributionsourcepackagerelease import (
@@ -81,23 +76,22 @@
@property
def publishing_history(self):
"""See IDistributionSourcePackageRelease."""
- return SourcePackagePublishingHistory.select("""
- DistroSeries.distribution = %s AND
- SourcePackagePublishingHistory.distroseries =
- DistroSeries.id AND
- SourcePackagePublishingHistory.archive IN %s AND
- SourcePackagePublishingHistory.sourcepackagerelease = %s
- """ % sqlvalues(self.distribution,
- self.distribution.all_distro_archive_ids,
- self.sourcepackagerelease),
- clauseTables=['DistroSeries'],
- orderBy='-datecreated')
+ from lp.registry.model.distroseries import DistroSeries
+ res = Store.of(self.distribution).find(
+ SourcePackagePublishingHistory,
+ SourcePackagePublishingHistory.archiveID.is_in(
+ self.distribution.all_distro_archive_ids),
+ SourcePackagePublishingHistory.distroseriesID == DistroSeries.id,
+ DistroSeries.distribution == self.distribution,
+ SourcePackagePublishingHistory.sourcepackagerelease ==
+ self.sourcepackagerelease)
+ return res.order_by(
+ Desc(SourcePackagePublishingHistory.datecreated),
+ Desc(SourcePackagePublishingHistory.id))
@property
def builds(self):
"""See IDistributionSourcePackageRelease."""
- store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
-
# Import DistroArchSeries here to avoid circular imports.
from lp.soyuz.model.distroarchseries import (
DistroArchSeries)
@@ -119,7 +113,7 @@
# First, get all the builds built in a main archive (this will
# include new and failed builds.)
- builds_built_in_main_archives = store.find(
+ builds_built_in_main_archives = Store.of(self.distribution).find(
BinaryPackageBuild,
builds_for_distro_exprs,
PackageBuild.archive == Archive.id,
@@ -129,7 +123,7 @@
# main archive... this will include many of those in the above
# query, but not the new/failed ones. It will also include
# ppa builds that have been published in main archives.
- builds_published_in_main_archives = store.find(
+ builds_published_in_main_archives = Store.of(self.distribution).find(
BinaryPackageBuild,
builds_for_distro_exprs,
BinaryPackageRelease.build == BinaryPackageBuild.id,
@@ -164,7 +158,6 @@
from lp.soyuz.model.distroarchseries import DistroArchSeries
from lp.soyuz.model.distroseriespackagecache import (
DistroSeriesPackageCache)
- store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
archive_ids = list(self.distribution.all_distro_archive_ids)
result_row = (
SQL('DISTINCT ON(BinaryPackageName.name) 0 AS ignore'),
@@ -198,7 +191,7 @@
DistroSeriesPackageCache.binarypackagename ==
BinaryPackageName.id)))
- all_published = store.using(*tables).find(
+ all_published = Store.of(self.distribution).using(*tables).find(
result_row,
DistroSeries.distribution == self.distribution,
BinaryPackagePublishingHistory.archiveID.is_in(archive_ids),
=== modified file 'lib/lp/soyuz/model/distroseriessourcepackagerelease.py'
--- lib/lp/soyuz/model/distroseriessourcepackagerelease.py 2012-07-05 22:38:43 +0000
+++ lib/lp/soyuz/model/distroseriessourcepackagerelease.py 2012-11-21 08:32:19 +0000
@@ -21,15 +21,12 @@
)
from storm.store import Store
from zope.interface import implements
-from zope.security.proxy import removeSecurityProxy
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.services.database.decoratedresultset import DecoratedResultSet
-from lp.services.database.sqlbase import sqlvalues
from lp.soyuz.interfaces.distroseriessourcepackagerelease import (
IDistroSeriesSourcePackageRelease,
)
-from lp.soyuz.interfaces.publishing import active_publishing_status
from lp.soyuz.interfaces.sourcepackagerelease import ISourcePackageRelease
from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
from lp.soyuz.model.binarypackagename import BinaryPackageName
@@ -208,45 +205,20 @@
@property
def publishing_history(self):
"""See `IDistroSeriesSourcePackage`."""
- # sqlvalues bails on security proxied objects.
- archive_ids = removeSecurityProxy(
- self.distroseries.distribution.all_distro_archive_ids)
- return SourcePackagePublishingHistory.select("""
- distroseries = %s AND
- archive IN %s AND
- sourcepackagerelease = %s
- """ % sqlvalues(
- self.distroseries,
- archive_ids,
- self.sourcepackagerelease),
- orderBy='-datecreated')
+ res = Store.of(self.distroseries).find(
+ SourcePackagePublishingHistory,
+ SourcePackagePublishingHistory.archiveID.is_in(
+ self.distroseries.distribution.all_distro_archive_ids),
+ SourcePackagePublishingHistory.distroseries == self.distroseries,
+ SourcePackagePublishingHistory.sourcepackagerelease ==
+ self.sourcepackagerelease)
+ return res.order_by(
+ Desc(SourcePackagePublishingHistory.datecreated),
+ Desc(SourcePackagePublishingHistory.id))
@property
def current_publishing_record(self):
"""An internal property used by methods of this class to know where
this release is or was published.
"""
- pub_hist = self.publishing_history
- try:
- return pub_hist[0]
- except IndexError:
- return None
-
- @property
- def current_published(self):
- """See `IDistroArchSeriesSourcePackage`."""
- # Retrieve current publishing info
- archive_ids = removeSecurityProxy(
- self.distroseries.distribution.all_distro_archive_ids)
- current = SourcePackagePublishingHistory.selectFirst("""
- distroseries = %s AND
- archive IN %s AND
- sourcepackagerelease = %s AND
- status IN %s
- """ % sqlvalues(self.distroseries,
- archive_ids,
- self.sourcepackagerelease,
- active_publishing_status),
- orderBy=['-datecreated', '-id'])
-
- return current
+ return self.publishing_history.first()
=== modified file 'lib/lp/soyuz/templates/distributionsourcepackage-portlet-pub-details.pt'
--- lib/lp/soyuz/templates/distributionsourcepackage-portlet-pub-details.pt 2012-07-06 06:02:33 +0000
+++ lib/lp/soyuz/templates/distributionsourcepackage-portlet-pub-details.pt 2012-11-21 08:32:19 +0000
@@ -14,7 +14,7 @@
<div tal:define="cr context/currentrelease">
- <div tal:condition="context/current_publishing_records">
+ <div tal:condition="not: context/current_publishing_records/is_empty">
<tal:block repeat="item view/all_published_in_active_distroseries">
<b><tal:suite replace="item/suite"/></b>
<tal:description replace="item/description"/>
@@ -22,7 +22,7 @@
</tal:block>
</div>
- <div tal:condition="not: context/current_publishing_records">
+ <div tal:condition="context/current_publishing_records/is_empty">
This source is not published in
<tal:name replace="context/distribution/name/capitalize"/>
</div>
=== modified file 'lib/lp/soyuz/templates/distributionsourcepackage-publishinghistory.pt'
--- lib/lp/soyuz/templates/distributionsourcepackage-publishinghistory.pt 2012-02-01 15:31:32 +0000
+++ lib/lp/soyuz/templates/distributionsourcepackage-publishinghistory.pt 2012-11-21 08:32:19 +0000
@@ -29,7 +29,8 @@
<div metal:fill-slot="main">
- <div class="top-portlet">
+ <div class="top-portlet" tal:define="publications view/batchnav">
+ <tal:navigation content="structure publications/@@+navigation-links-upper" />
<table id="publishing-summary" class="listing">
<thead>
<tr>
@@ -44,11 +45,12 @@
</tr>
</thead>
<tbody>
- <tal:block repeat="publishing context/publishing_history"
+ <tal:block repeat="publishing publications/currentBatch"
replace="structure publishing/@@+listing-summary">
</tal:block>
</tbody>
</table>
+ <tal:navigation content="structure publications/@@+navigation-links-lower" />
</div>
<p>
Follow ups
