launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09706
[Merge] lp:~wgrant/launchpad/dasbp-cleanup into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/dasbp-cleanup into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/dasbp-cleanup/+merge/113946
This branch Stormifies and shortens the publication queries on DistroArchSeriesBinaryPackage. I also tweaked them to use BinaryPackagePublishingHistory.binarypackagename rather than BinaryPackageRelease.binarypackagename, allowing them to use indices to avoid materialising tens of thousands of rows, cutting them from 5-150ms apiece to 0.5ms.
I also applied the BPR->BPPH change to guessPublishedSourcePackageName. And cleaned up some other horribleness I found on the way. Ugh.
--
https://code.launchpad.net/~wgrant/launchpad/dasbp-cleanup/+merge/113946
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/dasbp-cleanup into lp:launchpad.
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2012-07-05 08:52:03 +0000
+++ lib/lp/registry/model/distribution.py 2012-07-09 10:43:20 +0000
@@ -174,9 +174,9 @@
from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
from lp.soyuz.interfaces.buildrecords import IHasBuildRecords
+from lp.soyuz.interfaces.publishing import active_publishing_status
from lp.soyuz.model.archive import Archive
from lp.soyuz.model.binarypackagename import BinaryPackageName
-from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
from lp.soyuz.model.distributionsourcepackagerelease import (
DistributionSourcePackageRelease,
)
@@ -1244,10 +1244,9 @@
SourcePackagePublishingHistory.sourcepackagename ==
sourcepackagename,
SourcePackagePublishingHistory.status.is_in(
- (PackagePublishingStatus.PUBLISHED,
- PackagePublishingStatus.PENDING)
- )).order_by(
- Desc(SourcePackagePublishingHistory.id)).first()
+ active_publishing_status),
+ ).order_by(
+ Desc(SourcePackagePublishingHistory.id)).first()
if publishing is not None:
return sourcepackagename
@@ -1275,10 +1274,10 @@
# otherwise.)
BinaryPackagePublishingHistory.archiveID.is_in(
self.all_distro_archive_ids),
- BinaryPackagePublishingHistory.binarypackagereleaseID ==
- BinaryPackageRelease.id,
- BinaryPackageRelease.binarypackagename == binarypackagename,
- BinaryPackagePublishingHistory.dateremoved == None,
+ BinaryPackagePublishingHistory.binarypackagename ==
+ binarypackagename,
+ BinaryPackagePublishingHistory.status.is_in(
+ active_publishing_status),
).order_by(
Desc(BinaryPackagePublishingHistory.id)).first()
if bpph is not None:
@@ -1319,13 +1318,11 @@
orderBy = ['Archive.displayname']
if not show_inactive:
- active_statuses = (PackagePublishingStatus.PUBLISHED,
- PackagePublishingStatus.PENDING)
clauses.append("""
Archive.id IN (
SELECT archive FROM SourcepackagePublishingHistory
WHERE status IN %s)
- """ % sqlvalues(active_statuses))
+ """ % sqlvalues(active_publishing_status))
if text:
orderBy.insert(
=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py 2012-06-14 05:18:22 +0000
+++ lib/lp/registry/tests/test_distribution.py 2012-07-09 10:43:20 +0000
@@ -44,9 +44,9 @@
)
from lp.registry.interfaces.series import SeriesStatus
from lp.registry.tests.test_distroseries import CurrentSourceReleasesMixin
-from lp.services.database.constants import UTC_NOW
from lp.services.propertycache import get_property_cache
from lp.services.webapp import canonical_url
+from lp.soyuz.enums import PackagePublishingStatus
from lp.soyuz.interfaces.distributionsourcepackagerelease import (
IDistributionSourcePackageRelease,
)
@@ -143,7 +143,8 @@
distroseries = self.factory.makeDistroSeries()
self.factory.makeBinaryPackagePublishingHistory(
archive=distroseries.main_archive,
- binarypackagename='binary-package', dateremoved=UTC_NOW)
+ binarypackagename='binary-package',
+ status=PackagePublishingStatus.SUPERSEDED)
with ExpectedException(NotFoundError, ".*Binary package.*"):
distroseries.distribution.guessPublishedSourcePackageName(
'binary-package')
=== modified file 'lib/lp/soyuz/model/distroarchseriesbinarypackage.py'
--- lib/lp/soyuz/model/distroarchseriesbinarypackage.py 2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/model/distroarchseriesbinarypackage.py 2012-07-09 10:43:20 +0000
@@ -107,28 +107,29 @@
return self.cache.description
return None
+ def _getPublicationJoins(self):
+ return [
+ BinaryPackagePublishingHistory.distroarchseries
+ == self.distroarchseries,
+ BinaryPackagePublishingHistory.archiveID.is_in(
+ self.distribution.all_distro_archive_ids),
+ BinaryPackagePublishingHistory.binarypackagename
+ == self.binarypackagename,
+ BinaryPackagePublishingHistory.binarypackagereleaseID
+ == BinaryPackageRelease.id,
+ ]
+
def __getitem__(self, version):
"""See IDistroArchSeriesBinaryPackage."""
- query = """
- BinaryPackagePublishingHistory.distroarchseries = %s AND
- BinaryPackagePublishingHistory.archive IN %s AND
- BinaryPackagePublishingHistory.binarypackagerelease =
- BinaryPackageRelease.id AND
- BinaryPackageRelease.version = %s AND
- BinaryPackageRelease.binarypackagename = %s
- """ % sqlvalues(
- self.distroarchseries,
- self.distribution.all_distro_archive_ids,
- version,
- self.binarypackagename)
-
- bpph = BinaryPackagePublishingHistory.selectFirst(
- query, clauseTables=['binarypackagerelease'],
- orderBy=["-datecreated"])
+ bpph = IStore(BinaryPackagePublishingHistory).find(
+ BinaryPackagePublishingHistory,
+ BinaryPackageRelease.version == version,
+ *self._getPublicationJoins()
+ ).order_by(Desc(BinaryPackagePublishingHistory.datecreated)
+ ).first()
if bpph is None:
return None
-
return DistroArchSeriesBinaryPackageRelease(
distroarchseries=self.distroarchseries,
binarypackagerelease=bpph.binarypackagerelease)
@@ -136,19 +137,11 @@
@property
def releases(self):
"""See IDistroArchSeriesBinaryPackage."""
- ret = BinaryPackageRelease.select("""
- BinaryPackagePublishingHistory.distroarchseries = %s AND
- BinaryPackagePublishingHistory.archive IN %s AND
- BinaryPackagePublishingHistory.binarypackagerelease =
- BinaryPackageRelease.id AND
- BinaryPackageRelease.binarypackagename = %s
- """ % sqlvalues(
- self.distroarchseries,
- self.distribution.all_distro_archive_ids,
- self.binarypackagename),
- orderBy='-datecreated',
- distinct=True,
- clauseTables=['BinaryPackagePublishingHistory'])
+ ret = IStore(BinaryPackageRelease).find(
+ BinaryPackageRelease,
+ *self._getPublicationJoins()
+ ).order_by(Desc(BinaryPackageRelease.datecreated)
+ ).config(distinct=True)
result = []
versions = set()
for bpr in ret:
@@ -163,62 +156,33 @@
@property
def currentrelease(self):
"""See IDistroArchSeriesBinaryPackage."""
- releases = BinaryPackageRelease.select("""
- BinaryPackageRelease.binarypackagename = %s AND
- BinaryPackageRelease.id =
- BinaryPackagePublishingHistory.binarypackagerelease AND
- BinaryPackagePublishingHistory.distroarchseries = %s AND
- BinaryPackagePublishingHistory.archive IN %s AND
- BinaryPackagePublishingHistory.status = %s
- """ % sqlvalues(
- self.binarypackagename,
- self.distroarchseries,
- self.distribution.all_distro_archive_ids,
- PackagePublishingStatus.PUBLISHED),
- orderBy='-datecreated',
- limit=1,
- distinct=True,
- clauseTables=['BinaryPackagePublishingHistory'])
-
- # Listify to limit the SQL queries to one only.
- results = list(releases)
- if len(results) == 0:
+ try:
+ bpph = self.current_published
+ except NotFoundError:
return None
return DistroArchSeriesBinaryPackageRelease(
distroarchseries=self.distroarchseries,
- binarypackagerelease=results[0])
+ binarypackagerelease=bpph.binarypackagerelease)
@property
def publishing_history(self):
"""See IDistroArchSeriesBinaryPackage."""
return IStore(BinaryPackagePublishingHistory).find(
BinaryPackagePublishingHistory,
- BinaryPackageRelease.binarypackagename == self.binarypackagename,
- BinaryPackagePublishingHistory.distroarchseries ==
- self.distroarchseries,
- BinaryPackagePublishingHistory.archiveID.is_in(
- self.distribution.all_distro_archive_ids),
- BinaryPackagePublishingHistory.binarypackagereleaseID ==
- BinaryPackageRelease.id).config(distinct=True).order_by(
- Desc(BinaryPackagePublishingHistory.datecreated))
+ *self._getPublicationJoins()
+ ).config(distinct=True
+ ).order_by(Desc(BinaryPackagePublishingHistory.datecreated))
@property
def current_published(self):
"""See IDistroArchSeriesBinaryPackage."""
- current = BinaryPackagePublishingHistory.selectFirst("""
- BinaryPackagePublishingHistory.distroarchseries = %s AND
- BinaryPackagePublishingHistory.archive IN %s AND
- BinaryPackagePublishingHistory.binarypackagerelease =
- BinaryPackageRelease.id AND
- BinaryPackageRelease.binarypackagename = %s AND
- BinaryPackagePublishingHistory.status = %s
- """ % sqlvalues(
- self.distroarchseries,
- self.distribution.all_distro_archive_ids,
- self.binarypackagename,
- PackagePublishingStatus.PUBLISHED),
- clauseTables=['BinaryPackageRelease'],
- orderBy='-datecreated')
+ current = IStore(BinaryPackagePublishingHistory).find(
+ BinaryPackagePublishingHistory,
+ BinaryPackagePublishingHistory.status
+ == PackagePublishingStatus.PUBLISHED,
+ *self._getPublicationJoins()
+ ).order_by(Desc(BinaryPackagePublishingHistory.datecreated)
+ ).first()
if current is None:
raise NotFoundError("Binary package %s not published in %s/%s"
=== modified file 'lib/lp/soyuz/model/distroarchseriesbinarypackagerelease.py'
--- lib/lp/soyuz/model/distroarchseriesbinarypackagerelease.py 2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/model/distroarchseriesbinarypackagerelease.py 2012-07-09 10:43:20 +0000
@@ -15,10 +15,10 @@
from zope.interface import implements
from lp.services.database.sqlbase import sqlvalues
-from lp.soyuz.enums import PackagePublishingStatus
from lp.soyuz.interfaces.distroarchseriesbinarypackagerelease import (
IDistroArchSeriesBinaryPackageRelease,
)
+from lp.soyuz.interfaces.publishing import active_publishing_status
from lp.soyuz.model.distributionsourcepackagerelease import (
DistributionSourcePackageRelease,
)
@@ -84,11 +84,7 @@
@property
def current_publishing_record(self):
"""See `IDistroArchSeriesBinaryPackageRelease`."""
- status = [
- PackagePublishingStatus.PENDING,
- PackagePublishingStatus.PUBLISHED]
- record = self._latest_publishing_record(status=status)
- return record
+ return self._latest_publishing_record(status=active_publishing_status)
# XXX cprov 20071026: heavy queries should be moved near to the related
# content classes in order to be better maintained. In this specific case
Follow ups
