← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/launchpad/bug-730460-use-spph-factory into lp:launchpad

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-730460-use-spph-factory into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #730460 Create DistroSeriesDifferenceJobs
  https://bugs.launchpad.net/bugs/730460

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-730460-use-spph-factory/+merge/52515

Preparation for bug 730460: create a DistroSeriesDifferenceJob whenever we create a SourcePackagePublishingHistory.

Before we can do that, we must first channel all SPPH creation (outside of tests) through the existing factory—and that's what this branch does.  Basically nothing else.

Discussed with wgrant and SteveK, in particular whether there was any reason why gina passed ids instead of model objects to the SPPH constructor; whether it would be safe to change that; and whether it would be right for gina to create a corresponding DistributionSourcePackage if none existed.  The factory does that, so it's a permissions issue.

One test I touched slightly, just to remove a line of output from the test suite, and it revealed tons of lint.  I preferred not to mess with it further, since doctests must die anyway, but I could fix the lint if desired.

./lib/lp/soyuz/doc/soyuz-upload.txt
      30: source has bad indentation.
      44: source has bad indentation.
      51: source has bad indentation.
      71: source has bad indentation.
      87: source has bad indentation.
     121: source has bad indentation.
     133: source has bad indentation.
     141: source has bad indentation.
     145: source has bad indentation.
     152: source has bad indentation.
     162: source has bad indentation.
     166: source has bad indentation.
     170: source has bad indentation.
     184: source has bad indentation.
     190: source has bad indentation.
     213: source has bad indentation.
     228: source exceeds 78 characters.
     228: source has bad indentation.
     244: source has bad indentation.
     247: source has bad indentation.
     248: source exceeds 78 characters.
     254: source has bad indentation.
     258: source exceeds 78 characters.
     266: source has bad indentation.
     274: source has bad indentation.
     284: source has bad indentation.
     291: source has bad indentation.
     293: source exceeds 78 characters.
     305: source has bad indentation.
     315: source exceeds 78 characters.
     315: source has bad indentation.
     320: source has bad indentation.
     328: source has bad indentation.
     328: source has trailing whitespace.
     329: source has trailing whitespace.
     361: source has bad indentation.
     391: source has bad indentation.
     401: source has bad indentation.
     417: source has bad indentation.
     422: source exceeds 78 characters.
     436: source has bad indentation.
     445: source has bad indentation.
     448: source has bad indentation.
     453: source has bad indentation.
     459: source has bad indentation.
     464: source has bad indentation.
     471: source has bad indentation.
     476: source has bad indentation.
     490: source has bad indentation.
     495: source has bad indentation.
     502: source has bad indentation.
     522: source has bad indentation.
     529: source has bad indentation.
     530: source exceeds 78 characters.
     536: source has bad indentation.
     553: source has bad indentation.
     565: want exceeds 78 characters.
     566: want exceeds 78 characters.
     567: want exceeds 78 characters.
     573: source has bad indentation.
     582: source exceeds 78 characters.
     582: source has bad indentation.
     589: source has bad indentation.
     613: narrative has trailing whitespace.
     620: source has bad indentation.
     629: source has bad indentation.
     639: source has bad indentation.
     644: source has bad indentation.
     658: source has bad indentation.
     662: want exceeds 78 characters.
     663: want exceeds 78 characters.
     664: want exceeds 78 characters.
     669: source has bad indentation.
     673: source has bad indentation.
     682: source exceeds 78 characters.
     682: source has bad indentation.
     688: source has bad indentation.
     698: narrative uses a moin header.
     708: source has bad indentation.
     724: want exceeds 78 characters.
     727: want exceeds 78 characters.
     737: source has bad indentation.
     752: want exceeds 78 characters.
     753: want exceeds 78 characters.
     754: want exceeds 78 characters.
     755: want exceeds 78 characters.
     765: source has bad indentation.
     770: source has bad indentation.

And there's a few complaints about the "stand-alone comment" style, which I think is valid and so I'm not messing with it any further:

./lib/lp/soyuz/scripts/gina/handlers.py
     235: E301 expected 1 blank line, found 2
     260: E301 expected 1 blank line, found 2

The other lint I fixed.


Jeroen
-- 
https://code.launchpad.net/~jtv/launchpad/bug-730460-use-spph-factory/+merge/52515
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-730460-use-spph-factory into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-03-07 07:27:56 +0000
+++ database/schema/security.cfg	2011-03-08 06:06:23 +0000
@@ -831,6 +831,7 @@
 public.archive                                  = SELECT, UPDATE
 public.archivearch                              = SELECT, UPDATE
 public.distribution                             = SELECT
+public.distributionsourcepackage                = SELECT, INSERT
 public.packagediff                              = SELECT, INSERT, UPDATE
 public.binarypackagepublishinghistory           = SELECT, INSERT, UPDATE, DELETE
 public.sourcepackagepublishinghistory           = SELECT, INSERT, UPDATE, DELETE

=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py	2011-01-21 08:12:29 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py	2011-03-08 06:06:23 +0000
@@ -572,14 +572,15 @@
 
         Only create a record for primary archives (i.e. not for PPAs).
         """
+        if spph.archive.purpose != ArchivePurpose.PRIMARY:
+            return
+
+        distribution = spph.distroseries.distribution
         sourcepackagename = spph.sourcepackagerelease.sourcepackagename
-        distribution = spph.distroseries.distribution
-
-        if spph.archive.purpose == ArchivePurpose.PRIMARY:
-            dsp = cls._get(distribution, sourcepackagename)
-            if dsp is None:
-                cls._new(distribution, sourcepackagename,
-                         is_upstream_link_allowed(spph))
+        dsp = cls._get(distribution, sourcepackagename)
+        if dsp is None:
+            upstream_link_allowed = is_upstream_link_allowed(spph)
+            cls._new(distribution, sourcepackagename, upstream_link_allowed)
 
 
 class DistributionSourcePackageInDatabase(Storm):

=== modified file 'lib/lp/soyuz/doc/soyuz-upload.txt'
--- lib/lp/soyuz/doc/soyuz-upload.txt	2011-02-17 13:23:29 +0000
+++ lib/lp/soyuz/doc/soyuz-upload.txt	2011-03-08 06:06:23 +0000
@@ -529,7 +529,7 @@
 Now we process the accepted queue items, one more time.
 
   >>> script = os.path.join(config.root, "scripts", "process-accepted.py")
-  >>> process = subprocess.Popen([sys.executable, script, "ubuntutest"])
+  >>> process = subprocess.Popen([sys.executable, script, "ubuntutest", "-q"])
   >>> process.wait()
   0
 

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2011-03-07 07:00:25 +0000
+++ lib/lp/soyuz/model/publishing.py	2011-03-08 06:06:23 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0611,W0212
@@ -783,11 +783,9 @@
                 "Overriding component to '%s' failed because it would "
                 "require a new archive." % new_component.name)
 
-        return SourcePackagePublishingHistory(
+        return getUtility(IPublishingSet).newSourcePublication(
             distroseries=current.distroseries,
             sourcepackagerelease=current.sourcepackagerelease,
-            status=PackagePublishingStatus.PENDING,
-            datecreated=UTC_NOW,
             pocket=current.pocket,
             component=new_component,
             section=new_section,
@@ -1409,6 +1407,10 @@
                              distroseries, component, section, pocket,
                              ancestor=None):
         """See `IPublishingSet`."""
+        # Avoid circular import.
+        from lp.registry.model.distributionsourcepackage import (
+            DistributionSourcePackage)
+
         pub = SourcePackagePublishingHistory(
             distroseries=distroseries,
             pocket=pocket,
@@ -1419,9 +1421,6 @@
             status=PackagePublishingStatus.PENDING,
             datecreated=UTC_NOW,
             ancestor=ancestor)
-        # Import here to prevent import loop.
-        from lp.registry.model.distributionsourcepackage import (
-            DistributionSourcePackage)
         DistributionSourcePackage.ensure(pub)
         return pub
 

=== modified file 'lib/lp/soyuz/scripts/gina/handlers.py'
--- lib/lp/soyuz/scripts/gina/handlers.py	2011-01-02 22:32:22 +0000
+++ lib/lp/soyuz/scripts/gina/handlers.py	2011-03-08 06:06:23 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Gina db handlers.
@@ -54,6 +54,7 @@
     )
 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
 from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
+from lp.soyuz.interfaces.publishing import IPublishingSet
 from lp.soyuz.model.component import Component
 from lp.soyuz.model.files import (
     BinaryPackageFile,
@@ -160,6 +161,7 @@
 
     This class is used to handle the import process.
     """
+
     def __init__(self, ztm, distro_name, distroseries_name, dry_run,
                  ktdb, archive_root, keyrings, pocket, component_override):
         self.dry_run = dry_run
@@ -451,6 +453,7 @@
     This class has methods to make the sourcepackagerelease access
     on the launchpad db a little easier.
     """
+
     def __init__(self, KTDB, archive_root, keyrings, pocket,
                  component_override):
         self.distro_handler = DistroHandler()
@@ -693,10 +696,6 @@
 
     def publish(self, sourcepackagerelease, spdata):
         """Create the publishing entry on db if does not exist."""
-        # Avoid circular import.
-        from lp.soyuz.model.publishing import (
-            SourcePackagePublishingHistory)
-
         # Check if the sprelease is already published and if so, just
         # report it.
 
@@ -724,19 +723,15 @@
                          source_publishinghistory.status.title)
                 return
 
-        # Create the Publishing entry with status PENDING so that we can
-        # republish this later into a Soyuz archive.
-        entry = SourcePackagePublishingHistory(
-            distroseries=self.distroseries.id,
-            sourcepackagerelease=sourcepackagerelease.id,
-            status=PackagePublishingStatus.PENDING,
-            component=component.id,
-            section=section.id,
-            datecreated=UTC_NOW,
-            datepublished=UTC_NOW,
+        # 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,
+            component=component,
+            section=section,
             pocket=self.pocket,
-            archive=archive
-            )
+            archive=archive)
         log.info('Source package %s (%s) published' % (
             entry.sourcepackagerelease.sourcepackagename.name,
             entry.sourcepackagerelease.version))
@@ -765,6 +760,7 @@
 
 class BinaryPackageHandler:
     """Handler to deal with binarypackages."""
+
     def __init__(self, sphandler, archive_root, pocket):
         # Create other needed object handlers.
         self.distro_handler = DistroHandler()
@@ -796,14 +792,14 @@
         # shared between distribution releases, so match on the
         # distribution and the architecture tag of the distroarchseries
         # they were built for
-        query = ("BinaryPackageRelease.binarypackagename=%s AND "
-                 "BinaryPackageRelease.version=%s AND "
-                 "BinaryPackageRelease.build = BinaryPackageBuild.id AND "
-                 "BinaryPackageBuild.distro_arch_series = DistroArchSeries.id AND "
-                 "DistroArchSeries.distroseries = DistroSeries.id AND "
-                 "DistroSeries.distribution = %d" %
-                 (binaryname.id, quote(version),
-                  distroseries.distribution.id))
+        query = (
+            "BinaryPackageRelease.binarypackagename=%s AND "
+            "BinaryPackageRelease.version=%s AND "
+            "BinaryPackageRelease.build = BinaryPackageBuild.id AND "
+            "BinaryPackageBuild.distro_arch_series = DistroArchSeries.id AND "
+            "DistroArchSeries.distroseries = DistroSeries.id AND "
+            "DistroSeries.distribution = %d" %
+            (binaryname.id, quote(version), distroseries.distribution.id))
 
         if architecture != "all":
             query += ("AND DistroArchSeries.architecturetag = %s" %
@@ -881,7 +877,11 @@
 
         distroarchseries = distroarchinfo['distroarchseries']
         distribution = distroarchseries.distroseries.distribution
-        clauseTables = ["BinaryPackageBuild", "DistroArchSeries", "DistroSeries"]
+        clauseTables = [
+            "BinaryPackageBuild",
+            "DistroArchSeries",
+            "DistroSeries",
+            ]
 
         # XXX kiko 2006-02-03:
         # This method doesn't work for real bin-only NMUs that are
@@ -938,6 +938,7 @@
 
 class BinaryPackagePublisher:
     """Binarypackage publisher class."""
+
     def __init__(self, distroarchseries, pocket, component_override):
         self.distroarchseries = distroarchseries
         self.pocket = pocket
@@ -998,8 +999,7 @@
             supersededby = None,
             datemadepending = None,
             dateremoved = None,
-            archive=archive
-            )
+            archive=archive)
 
         log.info('BinaryPackage %s-%s published into %s.' % (
             binarypackage.binarypackagename.name, binarypackage.version,


Follow ups