launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03182
[Merge] lp:~jtv/launchpad/bug-745542 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-745542 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #745542 in Launchpad itself: "Limit parent-only DSD[J] generation to packagesets"
https://bugs.launchpad.net/launchpad/+bug/745542
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-745542/+merge/56106
See bug 745542: we don't want to generate DistroSeriesDifferences for packages that appear in parent distroseries (at least the ones with packagesets) unless they're in some packageset that the derived series also has.
There are rough edges to my approach, both discussed with the squad. One is that I filter in the DistroSeriesDifferenceJob runner, which means that we create more jobs than we otherwise would. I went this way to avoid gradually piling more work into the synchronous front end of the process, which we want to keep as light as possible. The other rough edge is that as packages are added to packagesets or packagesets are added to derived distroseries, any preceding source package publications are ignored and so packages only show up as distroseries differences when their next publication comes along.
So far there doesn't seem to be much interest in these scenarios; Julian is to talk to the stakeholders so we can decide whether we need to do anything more about them.
To test the changes, run:
{{{
./bin/test -vvc lp.soyuz.tests.test_distroseriesdifferencejob
}}}
For Q/A, the first thing we will observe is a bad effect: when you publish a package and then add it to a packageset, it doesn't show up among a derived series' differences until you create another SPPH.
The only lint: lines in security.cfg exceeding the 78-column limit. Duh.
Jeroen
--
https://code.launchpad.net/~jtv/launchpad/bug-745542/+merge/56106
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-745542 into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2011-04-01 00:47:03 +0000
+++ database/schema/security.cfg 2011-04-04 07:28:37 +0000
@@ -1076,6 +1076,7 @@
public.job = SELECT, UPDATE
public.libraryfilealias = SELECT
public.libraryfilecontent = SELECT
+public.packageset = SELECT
public.sourcepackagename = SELECT
public.sourcepackagepublishinghistory = SELECT
public.sourcepackagerelease = SELECT
=== modified file 'lib/lp/soyuz/interfaces/packageset.py'
--- lib/lp/soyuz/interfaces/packageset.py 2011-03-29 09:26:33 +0000
+++ lib/lp/soyuz/interfaces/packageset.py 2011-04-04 07:28:37 +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).
# pylint: disable-msg=E0211,E0213
@@ -235,9 +235,10 @@
@operation_returns_collection_of(Interface)
@export_read_operation()
def relatedSets():
- """Get all package sets related to this one.
+ """Get all other package sets in this set's `PackagesetGroup`.
- Return all package sets that are related to this one.
+ Returns all package sets that are related to this one, but not
+ this one itself.
:return: A (potentially empty) sequence of `IPackageset` instances.
"""
=== modified file 'lib/lp/soyuz/model/distroseriesdifferencejob.py'
--- lib/lp/soyuz/model/distroseriesdifferencejob.py 2011-03-18 03:39:58 +0000
+++ lib/lp/soyuz/model/distroseriesdifferencejob.py 2011-04-04 07:28:37 +0000
@@ -27,6 +27,7 @@
IDistroSeriesDifferenceJob,
IDistroSeriesDifferenceJobSource,
)
+from lp.soyuz.interfaces.packageset import IPackagesetSet
from lp.soyuz.model.distributionjob import (
DistributionJob,
DistributionJobDerived,
@@ -99,6 +100,12 @@
return find_waiting_jobs(distroseries, sourcepackagename).is_empty()
+def has_package(distroseries, sourcepackagename):
+ """Does `distroseries` have the given source package?"""
+ return not distroseries.getPublishedSources(
+ sourcepackagename, include_pending=True).is_empty()
+
+
class DistroSeriesDifferenceJob(DistributionJobDerived):
"""A `Job` type for creating/updating `DistroSeriesDifference`s."""
@@ -123,13 +130,42 @@
def sourcepackagename(self):
return SourcePackageName.get(self.metadata['sourcepackagename'])
+ def passesPackagesetFilter(self):
+ """Is this package of interest as far as packagesets are concerned?
+
+ If the parent series has packagesets, then packages that are
+ missing in the derived series are only of interest if they are
+ in a packageset that the derived series also has.
+ """
+ derived_series = self.distroseries
+ parent_series = derived_series.parent_series
+ if has_package(derived_series, self.sourcepackagename):
+ return True
+ if not has_package(parent_series, self.sourcepackagename):
+ return True
+ packagesetset = getUtility(IPackagesetSet)
+ if packagesetset.getBySeries(parent_series).is_empty():
+ # Parent series does not have packagesets, as would be the
+ # case for e.g. Debian. In that case, don't filter.
+ return True
+ parent_sets = packagesetset.setsIncludingSource(
+ self.sourcepackagename, distroseries=parent_series)
+ for parent_set in parent_sets:
+ for related_set in parent_set.relatedSets():
+ if related_set.distroseries == derived_series:
+ return True
+ return False
+
def run(self):
"""See `IRunnableJob`."""
+ if not self.passesPackagesetFilter():
+ return
+
store = IMasterStore(DistroSeriesDifference)
ds_diff = store.find(
- DistroSeriesDifference,
+ DistroSeriesDifference,
DistroSeriesDifference.derived_series == self.distroseries,
- DistroSeriesDifference.source_package_name ==
+ DistroSeriesDifference.source_package_name ==
self.sourcepackagename).one()
if ds_diff is None:
ds_diff = getUtility(IDistroSeriesDifferenceSource).new(
=== modified file 'lib/lp/soyuz/tests/test_distroseriesdifferencejob.py'
--- lib/lp/soyuz/tests/test_distroseriesdifferencejob.py 2011-03-28 07:29:51 +0000
+++ lib/lp/soyuz/tests/test_distroseriesdifferencejob.py 2011-04-04 07:28:37 +0000
@@ -198,7 +198,7 @@
self.assertEqual(0, len(jobs))
store = IMasterStore(DistroSeriesDifference)
ds_diff = store.find(
- DistroSeriesDifference,
+ DistroSeriesDifference,
DistroSeriesDifference.derived_series == derived_series,
DistroSeriesDifference.source_package_name == package)
self.assertEqual(1, ds_diff.count())
@@ -214,7 +214,7 @@
# The first job would have created a DSD for us.
store = IMasterStore(DistroSeriesDifference)
ds_diff = store.find(
- DistroSeriesDifference,
+ DistroSeriesDifference,
DistroSeriesDifference.derived_series == derived_series,
DistroSeriesDifference.source_package_name == package)
self.assertEqual(1, ds_diff.count())
@@ -224,11 +224,91 @@
job[0].start()
job[0].run()
ds_diff = store.find(
- DistroSeriesDifference,
+ DistroSeriesDifference,
DistroSeriesDifference.derived_series == derived_series,
DistroSeriesDifference.source_package_name == package)
self.assertEqual(1, ds_diff.count())
+ def test_packageset_filter_passes_inherited_packages(self):
+ derived_series = self.makeDerivedDistroSeries()
+ parent_series = derived_series.parent_series
+ # Parent must have a packageset or the filter will pass anyway.
+ self.factory.makePackageset(distroseries=parent_series)
+ package = self.factory.makeSourcePackageName()
+ # Package is not in the packageset _but_ both the parent and
+ # derived series both have it.
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=parent_series, sourcepackagename=package)
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=derived_series, sourcepackagename=package)
+ job = create_job(derived_series, package)
+ self.assertTrue(job.passesPackagesetFilter())
+
+ def test_packageset_filter_passes_packages_unique_to_derived_series(self):
+ derived_series = self.makeDerivedDistroSeries()
+ parent_series = derived_series.parent_series
+ # Parent must have a packageset or the filter will pass anyway.
+ self.factory.makePackageset(distroseries=parent_series)
+ package = self.factory.makeSourcePackageName()
+ # Package exists in the derived series but not in the parent
+ # series.
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=derived_series, sourcepackagename=package)
+ job = create_job(derived_series, package)
+ self.assertTrue(job.passesPackagesetFilter())
+
+ def test_packageset_filter_passes_all_if_parent_has_no_packagesets(self):
+ # Debian in particular has no packagesets. If the parent series
+ # has no packagesets, the packageset filter passes all packages.
+ derived_series = self.makeDerivedDistroSeries()
+ parent_series = derived_series.parent_series
+ package = self.factory.makeSourcePackageName()
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=parent_series, sourcepackagename=package)
+ job = create_job(derived_series, package)
+ self.assertTrue(job.passesPackagesetFilter())
+
+ def makeInheritedPackageSet(self, derived_series, packages=()):
+ """Simulate an inherited `Packageset`.
+
+ Creates a packageset in the parent that has an equivalent in
+ `derived_series`.
+ """
+ parent_series = derived_series.parent_series
+ parent_packageset = self.factory.makePackageset(
+ distroseries=parent_series, packages=packages)
+ derived_packageset = self.factory.makePackageset(
+ distroseries=derived_series, packages=packages,
+ name=parent_packageset.name, owner=parent_packageset.owner,
+ related_set=parent_packageset)
+
+ def test_packageset_filter_passes_package_in_inherited_packageset(self):
+ derived_series = self.makeDerivedDistroSeries()
+ parent_series = derived_series.parent_series
+ # Package is in a packageset on the parent that the derived
+ # series also has.
+ package = self.factory.makeSourcePackageName()
+ self.makeInheritedPackageSet(derived_series, [package])
+ # Package is in parent series and in a packageset that the
+ # derived series inherited.
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=parent_series, sourcepackagename=package)
+ job = create_job(derived_series, package)
+ self.assertTrue(job.passesPackagesetFilter())
+
+ def test_packageset_filter_blocks_unwanted_parent_package(self):
+ derived_series = self.makeDerivedDistroSeries()
+ parent_series = derived_series.parent_series
+ self.makeInheritedPackageSet(derived_series)
+ package = self.factory.makeSourcePackageName()
+ # Package is in the parent series but not in a packageset shared
+ # between the derived series and the parent series.
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=parent_series, sourcepackagename=package)
+ job = create_job(derived_series, package)
+ self.assertFalse(job.passesPackagesetFilter())
+
+
class TestDistroSeriesDifferenceJobEndToEnd(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
@@ -262,7 +342,7 @@
def findDSD(self, derived_series, source_package_name):
return self.store.find(
- DistroSeriesDifference,
+ DistroSeriesDifference,
DistroSeriesDifference.derived_series == derived_series,
DistroSeriesDifference.source_package_name ==
source_package_name)
@@ -348,7 +428,7 @@
self.runJob(jobs[0])
self.assertEqual(
DistroSeriesDifferenceStatus.RESOLVED, ds_diff[0].status)
-
+
def test_only_in_child(self):
# If a source package only exists in the child distroseries, the DSD
# is created with the right type.
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-03-31 12:53:27 +0000
+++ lib/lp/testing/factory.py 2011-04-04 07:28:37 +0000
@@ -3617,7 +3617,7 @@
return getUtility(ISectionSet).ensure(name)
def makePackageset(self, name=None, description=None, owner=None,
- packages=(), distroseries=None):
+ packages=(), distroseries=None, related_set=None):
"""Make an `IPackageset`."""
if name is None:
name = self.getUniqueString(u'package-set-name')
@@ -3630,7 +3630,8 @@
ps_set = getUtility(IPackagesetSet)
package_set = run_with_login(
techboard.teamowner,
- lambda: ps_set.new(name, description, owner, distroseries))
+ lambda: ps_set.new(
+ name, description, owner, distroseries, related_set))
run_with_login(owner, lambda: package_set.add(packages))
return package_set
