launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/dsdj-issues into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/dsdj-issues/+merge/54650

Correct some issues with creation of DSDJs.

Firstly, the gina DB user wasn't able to query or create them, leading to crashes when run with the feature flag enabled.

Secondly, DSDJs would be created for PPA uploads (or SPPH deletion in a PPA), which is wrong.
-- 
https://code.launchpad.net/~stevenk/launchpad/dsdj-issues/+merge/54650
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/dsdj-issues into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-03-22 01:15:32 +0000
+++ database/schema/security.cfg	2011-03-24 05:30:58 +0000
@@ -837,6 +837,7 @@
 public.archive                                  = SELECT, UPDATE
 public.archivearch                              = SELECT, UPDATE
 public.distribution                             = SELECT
+public.distributionjob                          = SELECT, INSERT
 public.distributionsourcepackage                = SELECT, INSERT
 public.packagediff                              = SELECT, INSERT, UPDATE
 public.binarypackagepublishinghistory           = SELECT, INSERT, UPDATE, DELETE

=== modified file 'lib/lp/soyuz/doc/gina.txt'
--- lib/lp/soyuz/doc/gina.txt	2010-11-23 10:51:58 +0000
+++ lib/lp/soyuz/doc/gina.txt	2011-03-24 05:30:58 +0000
@@ -50,6 +50,13 @@
     ...     hoary, celebs.launchpad_developers)
     >>> login(ANONYMOUS)
 
+    # Enable the Derived Series feature flag, to make sure gina likes it.
+    >>> from lp.services.features.testing import FeatureFixture
+    >>> from lp.soyuz.model.distroseriesdifferencejob import (
+    ...     FEATURE_FLAG_ENABLE_MODULE,
+    ...     )
+    >>> fixture = FeatureFixture({FEATURE_FLAG_ENABLE_MODULE: u'on'})
+    >>> fixture.setUp()
     >>> pf = ProcessorFamily.selectOneBy(name="x86")
     >>> breezy_i386 = DistroArchSeries(distroseries=breezy,
     ...                         processorfamily=pf,
@@ -176,6 +183,7 @@
     >>> proc.wait()
     0
     >>> transaction.commit()
+    >>> fixture.cleanUp()
 
 === Testing Source Package Results ===
 

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2011-03-23 01:10:23 +0000
+++ lib/lp/soyuz/model/publishing.py	2011-03-24 05:30:58 +0000
@@ -337,10 +337,11 @@
         self.removed_by = removed_by
         self.removal_comment = removal_comment
         if ISourcePackagePublishingHistory.providedBy(self):
-            dsd_job_source = getUtility(IDistroSeriesDifferenceJobSource)
-            dsd_job_source.createForPackagePublication(
-                self.distroseries,
-                self.sourcepackagerelease.sourcepackagename)
+            if self.archive == self.distroseries.main_archive:
+                dsd_job_source = getUtility(IDistroSeriesDifferenceJobSource)
+                dsd_job_source.createForPackagePublication(
+                    self.distroseries,
+                    self.sourcepackagerelease.sourcepackagename)
 
     def requestObsolescence(self):
         """See `IArchivePublisher`."""
@@ -1431,9 +1432,10 @@
             ancestor=ancestor)
         DistributionSourcePackage.ensure(pub)
 
-        dsd_job_source = getUtility(IDistroSeriesDifferenceJobSource)
-        dsd_job_source.createForPackagePublication(
-            distroseries, sourcepackagerelease.sourcepackagename)
+        if archive == distroseries.main_archive:
+            dsd_job_source = getUtility(IDistroSeriesDifferenceJobSource)
+            dsd_job_source.createForPackagePublication(
+                distroseries, sourcepackagerelease.sourcepackagename)
         return pub
 
     def getBuildsForSourceIds(

=== modified file 'lib/lp/soyuz/tests/test_distroseriesdifferencejob.py'
--- lib/lp/soyuz/tests/test_distroseriesdifferencejob.py	2011-03-22 06:49:35 +0000
+++ lib/lp/soyuz/tests/test_distroseriesdifferencejob.py	2011-03-24 05:30:58 +0000
@@ -245,7 +245,10 @@
         return self.factory.makeDistroSeries(
             parent_series=self.factory.makeDistroSeries())
 
-    def createPublication(self, source_package_name, versions, distroseries):
+    def createPublication(self, source_package_name, versions, distroseries,
+                          archive=None):
+        if archive is None:
+            archive = distroseries.main_archive
         changelog_lfa = self.factory.makeChangelog(
             source_package_name.name, versions)
         transaction.commit() # Yay, librarian.
@@ -253,7 +256,7 @@
             sourcepackagename=source_package_name, version=versions[0],
             changelog=changelog_lfa)
         return self.factory.makeSourcePackagePublishingHistory(
-            sourcepackagerelease=spr, archive=distroseries.main_archive,
+            sourcepackagerelease=spr, archive=archive,
             distroseries=distroseries,
             status=PackagePublishingStatus.PUBLISHED)
 
@@ -417,6 +420,27 @@
             DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES,
             ds_diff[0].difference_type)
 
+    def test_no_job_for_PPA(self):
+        # If a source package is uploaded to a PPA, a job is not created.
+        derived_series = self.makeDerivedDistroSeries()
+        source_package_name = self.factory.makeSourcePackageName()
+        ppa = self.factory.makeArchive()
+        self.createPublication(
+            source_package_name, ['1.0-1'], derived_series, ppa)
+        jobs = find_waiting_jobs(derived_series, source_package_name)
+        self.assertEqual(0, jobs.count())
+
+    def test_no_job_for_PPA_with_deleted_source(self):
+        # If a source package is deleted from a PPA, no job is created.
+        derived_series = self.makeDerivedDistroSeries()
+        source_package_name = self.factory.makeSourcePackageName()
+        ppa = self.factory.makeArchive()
+        spph = self.createPublication(
+            source_package_name, ['1.0-1'], derived_series, ppa)
+        spph.requestDeletion(ppa.owner)
+        jobs = find_waiting_jobs(derived_series, source_package_name)
+        self.assertEqual(0, jobs.count())
+
 
 class TestDistroSeriesDifferenceJobPermissions(TestCaseWithFactory):
     """Database permissions test for `DistroSeriesDifferenceJob`."""