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))

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to