launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15360
[Merge] lp:~stevenk/launchpad/performant-getSourcesForDeletion into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/performant-getSourcesForDeletion into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #590523 in Launchpad itself: "Archive:+delete-packages times out"
https://bugs.launchpad.net/launchpad/+bug/590523
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/performant-getSourcesForDeletion/+merge/153704
Rewrite IArchive.getSourcesForDeletion() to be performant, and Storm-ify it. It now makes use of SPPH.scheduleddeletiondate, which required some test fiddling.
Perform a little bit of clean-up, like destroying the use of SQLConstant, but this branch was already net-negative just due to switching to a simpler query. Drop a unneeded database perm that was missed by a testfix of mine.
--
https://code.launchpad.net/~stevenk/launchpad/performant-getSourcesForDeletion/+merge/153704
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/performant-getSourcesForDeletion into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2013-03-12 05:51:28 +0000
+++ database/schema/security.cfg 2013-03-18 01:16:23 +0000
@@ -2197,7 +2197,6 @@
public.translationrelicensingagreement = SELECT, UPDATE
public.translator = SELECT, UPDATE
public.usertouseremail = SELECT, UPDATE
-public.validpersoncache = SELECT
public.vote = SELECT, UPDATE
public.votecast = SELECT, UPDATE
public.wikiname = SELECT, UPDATE
=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt 2013-02-20 05:43:59 +0000
+++ lib/lp/soyuz/doc/archive.txt 2013-03-18 01:16:23 +0000
@@ -573,8 +573,10 @@
>>> cprov_archive.number_of_sources
3
- >>> cprov_archive.getPublishedSources(
- ... name=u'cdrkit').first().supersede()
+ >>> cdrkit = cprov_archive.getPublishedSources(name=u'cdrkit').first()
+ >>> cdrkit.supersede()
+ >>> from lp.services.database.constants import UTC_NOW
+ >>> cdrkit.scheduleddeletiondate = UTC_NOW
>>> cprov_archive.number_of_sources
2
@@ -608,7 +610,7 @@
This method can optionally receive a source package name filter (SQL
LIKE) to restrict its result.
- >>> cprov_archive.getSourcesForDeletion(name='ice').count()
+ >>> cprov_archive.getSourcesForDeletion(name=u'ice').count()
1
If only the source publication is DELETED, leaving its binary behind,
@@ -622,29 +624,28 @@
>>> login("celso.providelo@xxxxxxxxxxxxx")
>>> removal_candidate.requestDeletion(cprov, 'go away !')
- >>> cprov_archive.getSourcesForDeletion(name='ice').count()
+ >>> cprov_archive.getSourcesForDeletion(name=u'ice').count()
1
The status filter can be used to only return sources that can be
deleted matching a given status.
>>> cprov_archive.getSourcesForDeletion(
- ... name='ice', status=PackagePublishingStatus.DELETED).count()
+ ... name=u'ice', status=PackagePublishingStatus.DELETED).count()
1
>>> cprov_archive.getSourcesForDeletion(
- ... name='ice', status=PackagePublishingStatus.PUBLISHED).count()
+ ... name=u'ice', status=PackagePublishingStatus.PUBLISHED).count()
0
The status filter can also be a sequence of status.
>>> irrelevant_status = (
... PackagePublishingStatus.SUPERSEDED,
- ... PackagePublishingStatus.DELETED,
- ... )
+ ... PackagePublishingStatus.DELETED)
>>> cprov_archive.getSourcesForDeletion(
- ... name='ice', status=irrelevant_status).count()
+ ... name=u'ice', status=irrelevant_status).count()
1
The series filter can be used to return only sources from a certain
@@ -655,20 +656,18 @@
>>> cprov_archive.getSourcesForDeletion(distroseries=hoary).count()
0
-The source publication is only excluded from 'deletion list' when its
-binary is also DELETED.
-
- >>> for bin in removal_candidate.getPublishedBinaries():
- ... bin.requestDeletion(cprov, 'go away !')
-
- >>> cprov_archive.getSourcesForDeletion(name='ice').count()
+The source publication is only excluded from 'deletion list' when it's
+scheduled deletion date is set.
+
+ >>> from zope.security.proxy import removeSecurityProxy
+ >>> removeSecurityProxy(removal_candidate).scheduleddeletiondate = UTC_NOW
+ >>> cprov_archive.getSourcesForDeletion(name=u'ice').count()
0
Flush the database caches to invalidate old caches from the
corresponding publishing Postgres views.
- >>> from lp.services.database.sqlbase import flush_database_caches
- >>> flush_database_caches()
+ >>> transaction.commit()
Build Lookup
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2013-03-07 01:09:59 +0000
+++ lib/lp/soyuz/model/archive.py 2013-03-18 01:16:23 +0000
@@ -23,7 +23,6 @@
IntCol,
StringCol,
)
-from sqlobject.sqlbuilder import SQLConstant
from storm.expr import (
And,
Desc,
@@ -75,6 +74,7 @@
from lp.registry.model.sourcepackagename import SourcePackageName
from lp.registry.model.teammembership import TeamParticipation
from lp.services.config import config
+from lp.services.database.bulk import load_related
from lp.services.database.constants import UTC_NOW
from lp.services.database.datetimecol import UtcDateTimeCol
from lp.services.database.decoratedresultset import DecoratedResultSet
@@ -87,7 +87,6 @@
from lp.services.database.lpstorm import ISlaveStore
from lp.services.database.sqlbase import (
cursor,
- quote,
quote_like,
SQLBase,
sqlvalues,
@@ -608,70 +607,36 @@
list(store.find(GPGKey, GPGKey.id.is_in(ids)))
return DecoratedResultSet(resultset, pre_iter_hook=eager_load)
- def getSourcesForDeletion(self, name=None, status=None,
- distroseries=None):
+ def getSourcesForDeletion(self, name=None, status=None, distroseries=None):
"""See `IArchive`."""
- clauses = ["""
- SourcePackagePublishingHistory.archive = %s AND
- SourcePackagePublishingHistory.sourcepackagerelease =
- SourcePackageRelease.id AND
- SourcePackagePublishingHistory.sourcepackagename =
- SourcePackageName.id
- """ % sqlvalues(self)]
-
- has_published_binaries_clause = """
- EXISTS (SELECT TRUE FROM
- BinaryPackagePublishingHistory bpph,
- BinaryPackageRelease bpr, BinaryPackageBuild
- WHERE
- bpph.archive = %s AND
- bpph.status = %s AND
- bpph.binarypackagerelease = bpr.id AND
- bpr.build = BinaryPackageBuild.id AND
- BinaryPackageBuild.source_package_release =
- SourcePackageRelease.id)
- """ % sqlvalues(self, PackagePublishingStatus.PUBLISHED)
-
- source_deletable_states = (
- PackagePublishingStatus.PENDING,
- PackagePublishingStatus.PUBLISHED,
- )
- clauses.append("""
- (%s OR SourcePackagePublishingHistory.status IN %s)
- """ % (has_published_binaries_clause,
- quote(source_deletable_states)))
-
- if status is not None:
+ clauses = [
+ SourcePackagePublishingHistory.archiveID == self.id,
+ SourcePackagePublishingHistory.sourcepackagereleaseID ==
+ SourcePackageRelease.id,
+ SourcePackagePublishingHistory.sourcepackagenameID ==
+ SourcePackageName.id,
+ SourcePackagePublishingHistory.scheduleddeletiondate == None]
+
+ if status:
try:
status = tuple(status)
except TypeError:
status = (status, )
- clauses.append("""
- SourcePackagePublishingHistory.status IN %s
- """ % sqlvalues(status))
-
- if distroseries is not None:
- clauses.append("""
- SourcePackagePublishingHistory.distroseries = %s
- """ % sqlvalues(distroseries))
-
- clauseTables = ['SourcePackageRelease', 'SourcePackageName']
-
- order_const = "SourcePackageRelease.version"
- desc_version_order = SQLConstant(order_const + " DESC")
- orderBy = ['SourcePackageName.name', desc_version_order,
- '-SourcePackagePublishingHistory.id']
-
- if name is not None:
- clauses.append("""
- SourcePackageName.name LIKE '%%' || %s || '%%'
- """ % quote_like(name))
-
- preJoins = ['sourcepackagerelease']
- sources = SourcePackagePublishingHistory.select(
- ' AND '.join(clauses), clauseTables=clauseTables, orderBy=orderBy,
- prejoins=preJoins)
-
+ clauses.append(SourcePackagePublishingHistory.status.is_in(status))
+
+ if distroseries:
+ clauses.append(
+ SourcePackagePublishingHistory.distroseriesID ==
+ distroseries.id)
+
+ if name:
+ clauses.append(SourcePackageName.name.contains_string(name))
+
+ sources = Store.of(self).find(
+ SourcePackagePublishingHistory, *clauses).order_by(
+ SourcePackageName.name, Desc(SourcePackageRelease.version),
+ Desc(SourcePackagePublishingHistory.id))
+ load_related(SourcePackageRelease, sources, ['sourcepackagereleaseID'])
return sources
@property
@@ -758,9 +723,7 @@
BinaryPackageRelease.version = %s
""" % sqlvalues(version))
elif ordered:
- order_const = "BinaryPackageRelease.version"
- desc_version_order = SQLConstant(order_const + " DESC")
- orderBy.insert(1, desc_version_order)
+ orderBy.insert(1, Desc(BinaryPackageRelease.version))
if status is not None:
try:
@@ -992,8 +955,7 @@
Component.name.is_in(components))
for (archive, not_used, pocket, components) in deps])
- store = ISlaveStore(BinaryPackagePublishingHistory)
- return store.find(
+ return ISlaveStore(BinaryPackagePublishingHistory).find(
BinaryPackagePublishingHistory,
BinaryPackageName.name == dep_name,
BinaryPackagePublishingHistory.binarypackagename ==
Follow ups
