launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04902
[Merge] lp:~jtv/launchpad/bug-844577 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-844577 into lp:launchpad with lp:~jtv/launchpad/re-830890 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #844577 in Launchpad itself: "Gina imports should be Published, not Pending"
https://bugs.launchpad.net/launchpad/+bug/844577
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-844577/+merge/74568
= Summary =
When Gina imports a package version (which would currently have to be a Debian source package release, but could be an Ubuntu-derived source or binary package release), it creates a BinaryPackagePublishingHistory or SourcePackagePublishingHistory (as the case may be) in Pending state. So basically all the Debian package releases we ever imported from Debian are basically sitting in Hades, waiting patiently for their time to return to the land of the living.
That time has come. We're making Gina run the Dominator (it's a long story) and this will only work on Published records.
== Proposed fix ==
Make Gina produce Published records instead. We'll also have to do some in-DB updates. Or more precisely, vast numbers of them.
== Pre-implementation notes ==
Discussed with William. The Pending status only made sense for the original Ubuntu import into Launchpad. Now, Published is the more appropriate status. (In practice it hasn't mattered much until now; you could call this part of a tech-debt fix).
== Implementation details ==
The way it's done is nastily asymmetric between sources and binaries: in the source case we use the IPublishingSet utility to create a Pending SPPH, and then use its API to make it Published. In the binary case, the status is passed directly to the constructor. But if Soyuz people like it this way, this is not the time to argue.
You'll also note a few minor (and largely irrelevant) cleanups of how statuses are mentioned: some unused variables that were used as shorthand for PackagePublishingStatus.* but which were not in use, and some literal spellings of what is clearly meant to be active_publishing_status. No functional changes. There's also an interesting 6-year-old XXX from someone who doesn't work here any more, about how to initialize a variable that wasn't being used. If we haven't noticed any problems with it, either things are fine as they are or the comment clearly isn't doing the trick.
== Tests ==
{{{
./bin/test -vvc lp.soyuz.scripts.tests.test_gina
}}}
== Demo and Q/A ==
Running Gina should still work. But now it will create SPPHs and BPPHs in Published status, rather than Pending.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/archivepublisher/domination.py
lib/lp/soyuz/scripts/gina/dominate.py
lib/lp/soyuz/model/publishing.py
lib/lp/soyuz/interfaces/publishing.py
scripts/gina.py
lib/lp/archivepublisher/tests/test_dominator.py
lib/lp/soyuz/scripts/tests/test_gina.py
lib/lp/soyuz/scripts/gina/handlers.py
./lib/lp/soyuz/interfaces/publishing.py
381: E261 at least two spaces before inline comment
478: E261 at least two spaces before inline comment
511: E261 at least two spaces before inline comment
681: E261 at least two spaces before inline comment
767: E261 at least two spaces before inline comment
./scripts/gina.py
26: '_pythonpath' imported but unused
--
https://code.launchpad.net/~jtv/launchpad/bug-844577/+merge/74568
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-844577 into lp:launchpad.
=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py 2011-09-08 09:37:32 +0000
+++ lib/lp/soyuz/model/publishing.py 2011-09-08 09:37:32 +0000
@@ -114,10 +114,6 @@
from lp.soyuz.scripts.changeoverride import ArchiveOverriderError
-PENDING = PackagePublishingStatus.PENDING
-PUBLISHED = PackagePublishingStatus.PUBLISHED
-
-
# XXX cprov 2006-08-18: move it away, perhaps archivepublisher/pool.py
def makePoolPath(source_name, component_name):
@@ -1081,7 +1077,7 @@
return IMasterStore(BinaryPackagePublishingHistory).find(
BinaryPackagePublishingHistory,
BinaryPackagePublishingHistory.status.is_in(
- [PUBLISHED, PENDING]),
+ active_publishing_status),
BinaryPackagePublishingHistory.distroarchseriesID.is_in(
available_architectures),
binarypackagerelease=self.binarypackagerelease,
@@ -1101,7 +1097,7 @@
return IMasterStore(BinaryPackagePublishingHistory).find(
BinaryPackagePublishingHistory,
BinaryPackagePublishingHistory.status.is_in(
- [PUBLISHED, PENDING]),
+ active_publishing_status),
BinaryPackagePublishingHistory.distroarchseries ==
self.distroarchseries,
binarypackagerelease=self.binarypackagerelease.debug_package,
=== modified file 'lib/lp/soyuz/scripts/gina/handlers.py'
--- lib/lp/soyuz/scripts/gina/handlers.py 2011-05-20 07:43:58 +0000
+++ lib/lp/soyuz/scripts/gina/handlers.py 2011-09-08 09:37:32 +0000
@@ -35,12 +35,12 @@
from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
from canonical.launchpad.scripts import log
from lp.archivepublisher.diskpool import poolify
+from lp.archiveuploader.changesfile import ChangesFile
from lp.archiveuploader.tagfiles import parse_tagfile
from lp.archiveuploader.utils import (
determine_binary_file_type,
determine_source_file_type,
)
-from lp.archiveuploader.changesfile import ChangesFile
from lp.buildmaster.enums import BuildStatus
from lp.registry.interfaces.person import (
IPersonSet,
@@ -54,7 +54,10 @@
)
from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
-from lp.soyuz.interfaces.publishing import IPublishingSet
+from lp.soyuz.interfaces.publishing import (
+ active_publishing_status,
+ IPublishingSet,
+ )
from lp.soyuz.model.component import Component
from lp.soyuz.model.files import (
BinaryPackageFile,
@@ -723,8 +726,6 @@
source_publishinghistory.status.title)
return
- # Create the Publishing entry, with status PENDING so that we
- # can republish this later into a Soyuz archive.
entry = getUtility(IPublishingSet).newSourcePublication(
distroseries=self.distroseries,
sourcepackagerelease=sourcepackagerelease,
@@ -732,6 +733,7 @@
section=section,
pocket=self.pocket,
archive=archive)
+ entry.setPublished()
log.info('Source package %s (%s) published' % (
entry.sourcepackagerelease.sourcepackagename.name,
entry.sourcepackagerelease.version))
@@ -742,16 +744,14 @@
from lp.soyuz.model.publishing import (
SourcePackagePublishingHistory)
- ret = SourcePackagePublishingHistory.select(
- """sourcepackagerelease = %s
- AND distroseries = %s
- AND archive = %s
- AND status in (%s, %s)""" %
- sqlvalues(sourcepackagerelease, self.distroseries,
- self.distroseries.main_archive,
- PackagePublishingStatus.PUBLISHED,
- PackagePublishingStatus.PENDING),
- orderBy=["-datecreated"])
+ ret = SourcePackagePublishingHistory.select("""
+ sourcepackagerelease = %s AND
+ distroseries = %s AND
+ archive = %s AND
+ status in %s""" % sqlvalues(
+ sourcepackagerelease, self.distroseries,
+ self.distroseries.main_archive, active_publishing_status),
+ orderBy=["-datecreated"])
ret = list(ret)
if ret:
return ret[0]
@@ -917,14 +917,6 @@
"for package %s (%s)" %
(build.id, binary.package, binary.version))
else:
-
- # XXX Debonzi 2005-05-16: Check it later
- # if bin.gpg_signing_key_owner:
- # key = self.getGPGKey(bin.gpg_signing_key,
- # *bin.gpg_signing_key_owner)
- # else:
- key = None
-
processor = distroarchinfo['processor']
build = getUtility(IBinaryPackageBuildSet).new(
processor=processor.id,
@@ -948,8 +940,7 @@
def publish(self, binarypackage, bpdata):
"""Create the publishing entry on db if does not exist."""
# Avoid circular imports.
- from lp.soyuz.model.publishing import (
- BinaryPackagePublishingHistory)
+ from lp.soyuz.model.publishing import BinaryPackagePublishingHistory
# These need to be pulled from the binary package data, not the
# binary package release: the data represents data from /this
@@ -983,22 +974,20 @@
binpkg_publishinghistory.status.title)
return
-
- # Create the Publishing entry with status PENDING.
BinaryPackagePublishingHistory(
- binarypackagerelease = binarypackage.id,
- component = component.id,
- section = section.id,
- priority = priority,
- distroarchseries = self.distroarchseries.id,
- status = PackagePublishingStatus.PENDING,
- datecreated = UTC_NOW,
- datepublished = UTC_NOW,
- pocket = self.pocket,
- datesuperseded = None,
- supersededby = None,
- datemadepending = None,
- dateremoved = None,
+ binarypackagerelease=binarypackage.id,
+ component=component.id,
+ section=section.id,
+ priority=priority,
+ distroarchseries=self.distroarchseries.id,
+ status=PackagePublishingStatus.PUBLISHED,
+ datecreated=UTC_NOW,
+ datepublished=UTC_NOW,
+ pocket=self.pocket,
+ datesuperseded=None,
+ supersededby=None,
+ datemadepending=None,
+ dateremoved=None,
archive=archive)
log.info('BinaryPackage %s-%s published into %s.' % (
@@ -1008,19 +997,16 @@
def _checkPublishing(self, binarypackage):
"""Query for the publishing entry"""
# Avoid circular imports.
- from lp.soyuz.model.publishing import (
- BinaryPackagePublishingHistory)
+ from lp.soyuz.model.publishing import BinaryPackagePublishingHistory
- ret = BinaryPackagePublishingHistory.select(
- """binarypackagerelease = %s
- AND distroarchseries = %s
- AND archive = %s
- AND status in (%s, %s)""" %
- sqlvalues(binarypackage, self.distroarchseries,
- self.distroarchseries.main_archive,
- PackagePublishingStatus.PUBLISHED,
- PackagePublishingStatus.PENDING),
- orderBy=["-datecreated"])
+ ret = BinaryPackagePublishingHistory.select("""
+ binarypackagerelease = %s AND
+ distroarchseries = %s AND
+ archive = %s AND
+ status in %s""" % sqlvalues(
+ binarypackage, self.distroarchseries,
+ self.distroarchseries.main_archive, active_publishing_status),
+ orderBy=["-datecreated"])
ret = list(ret)
if ret:
return ret[0]
=== modified file 'lib/lp/soyuz/scripts/tests/test_gina.py'
--- lib/lp/soyuz/scripts/tests/test_gina.py 2011-09-08 09:37:32 +0000
+++ lib/lp/soyuz/scripts/tests/test_gina.py 2011-09-08 09:37:32 +0000
@@ -10,6 +10,14 @@
from lp.soyuz.enums import PackagePublishingStatus
from lp.soyuz.scripts.gina.dominate import dominate_imported_source_packages
import lp.soyuz.scripts.gina.handlers
+from lp.soyuz.scripts.gina.handlers import (
+ BinaryPackagePublisher,
+ SourcePackagePublisher,
+ )
+from lp.soyuz.scripts.gina.packages import (
+ BinaryPackageData,
+ SourcePackageData,
+ )
from lp.testing import TestCaseWithFactory
@@ -70,6 +78,50 @@
[pub.status for pub in pubs])
+class TestSourcePackagePublisher(TestCaseWithFactory):
+
+ layer = ZopelessDatabaseLayer
+
+ def test_publish_creates_published_publication(self):
+ maintainer = self.factory.makePerson()
+ series = self.factory.makeDistroSeries()
+ section = self.factory.makeSection()
+ pocket = PackagePublishingPocket.RELEASE
+ spr = self.factory.makeSourcePackageRelease()
+
+ publisher = SourcePackagePublisher(series, pocket, None)
+ publisher.publish(spr, SourcePackageData(
+ component='main', section=section.name, version='1.0',
+ maintainer=maintainer.preferredemail, architecture='all',
+ files='foo.py', binaries='foo.py'))
+
+ [spph] = series.main_archive.getPublishedSources()
+ self.assertEqual(PackagePublishingStatus.PUBLISHED, spph.status)
+
+
+class TestBinaryPackagePublisher(TestCaseWithFactory):
+
+ layer = ZopelessDatabaseLayer
+
+ def test_publish_creates_published_publication(self):
+ maintainer = self.factory.makePerson()
+ series = self.factory.makeDistroArchSeries()
+ section = self.factory.makeSection()
+ pocket = PackagePublishingPocket.RELEASE
+ bpr = self.factory.makeBinaryPackageRelease()
+
+ publisher = BinaryPackagePublisher(series, pocket, None)
+ publisher.publish(bpr, BinaryPackageData(
+ component='main', section=section.name, version='1.0',
+ maintainer=maintainer.preferredemail, architecture='all',
+ files='foo.py', binaries='foo.py', size=128, installed_size=1024,
+ md5sum='e83b5dd68079d727a494a469d40dc8db', description='test',
+ summary='Test!'))
+
+ [bpph] = series.main_archive.getAllPublishedBinaries()
+ self.assertEqual(PackagePublishingStatus.PUBLISHED, bpph.status)
+
+
def test_suite():
suite = TestLoader().loadTestsFromName(__name__)
suite.addTest(DocTestSuite(lp.soyuz.scripts.gina.handlers))
Follow ups
