launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04299
[Merge] lp:~rvb/launchpad/dsd-init-bug-808681 into lp:launchpad
Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/dsd-init-bug-808681 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #808681 in Launchpad itself: "Post-first series initializations don't set up the DistroSeriesDifferences rows"
https://bugs.launchpad.net/launchpad/+bug/808681
For more details, see:
https://code.launchpad.net/~rvb/launchpad/dsd-init-bug-808681/+merge/68375
This branch adds the creation of the DSDs as a part of the initialization process. DSD are created using the methods in lib/lp/registry/scripts/populate_distroseriesdiff.py if the parents have changed (compared to the previous_series' parent) or if only specific packagesets have been selected for the initialization. If not, then a simple copy from previous_series' DSDs is done. (This should speed up the initialization of Ubuntu series.)
= Details =
- New authorizations have been added to database/schema/security.cfg to allow scripts to manipulate the distroseriesdifference table.
- lib/lp/registry/scripts/populate_distroseriesdiff.py has been reworked a bit to be able to call it's methods from outside the script.
= Tests =
./bin/test -vvc test_initialize_distroseries test__has_same_parents_as_previous_series_explicit
./bin/test -vvc test_initialize_distroseries test__has_same_parents_as_previous_series_implicit
./bin/test -vvc test_initialize_distroseries test_not__has_same_parents_as_previous_series
./bin/test -vvc test_initialize_distroseries test_initialization_copy_dsds
./bin/test -vvc test_initialize_distroseries test_initialization_compute_dsds
./bin/test -vvc test_initialize_distroseries test_initialization_compute_dsds_specific_packagesets
= QA =
On DF, make a post-first initialization of a series from multiple parents without changing the (pre-selected on the +initseries page) parents. Make sure that lp.net/distro/series/+localpackagediffs is populated and is the same as lp.net/distro/previous_series/+localpackagediffs.
On DF, make a post-first initialization of a series from multiple parents and add a new parent (compared to previous_series' parents). Make sure that lp.net/distro/series/+localpackagediffs is populated with diffs from the series and all of it's parents.
--
https://code.launchpad.net/~rvb/launchpad/dsd-init-bug-808681/+merge/68375
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/dsd-init-bug-808681 into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2011-07-15 15:35:03 +0000
+++ database/schema/security.cfg 2011-07-19 12:56:28 +0000
@@ -857,6 +857,7 @@
public.cve = SELECT, INSERT
public.distributionjob = SELECT, INSERT, DELETE
public.distributionsourcepackage = SELECT, INSERT, UPDATE
+public.distroseriesdifference = SELECT, INSERT, UPDATE
public.distroseriesparent = SELECT, INSERT, UPDATE
public.flatpackagesetinclusion = SELECT, INSERT, UPDATE, DELETE
public.gpgkey = SELECT, INSERT, UPDATE
@@ -979,6 +980,7 @@
public.distributionsourcepackage = SELECT, INSERT
public.distroarchseries = SELECT, INSERT
public.distroseries = SELECT, UPDATE
+public.distroseriesdifference = SELECT, INSERT, UPDATE
public.distroseriesparent = SELECT, INSERT, UPDATE
public.flatpackagesetinclusion = SELECT, INSERT
public.gpgkey = SELECT
=== modified file 'lib/lp/registry/scripts/populate_distroseriesdiff.py'
--- lib/lp/registry/scripts/populate_distroseriesdiff.py 2011-05-20 14:36:26 +0000
+++ lib/lp/registry/scripts/populate_distroseriesdiff.py 2011-07-19 12:56:28 +0000
@@ -14,6 +14,8 @@
__metaclass__ = type
__all__ = [
'PopulateDistroSeriesDiff',
+ 'populate_distroseriesdiff',
+ 'update_distroseriesdiff',
]
from collections import defaultdict
@@ -21,6 +23,7 @@
Option,
OptionValueError,
)
+
from storm.locals import ClassAlias
import transaction
from zope.component import getUtility
@@ -30,17 +33,18 @@
quote_identifier,
)
from canonical.launchpad.interfaces.lpstorm import IStore
+from canonical.launchpad.utilities.looptuner import TunableLoop
from lp.registry.enum import (
DistroSeriesDifferenceStatus,
DistroSeriesDifferenceType,
)
-from canonical.launchpad.utilities.looptuner import TunableLoop
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.distroseriesparent import IDistroSeriesParentSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.model.distroseries import DistroSeries
from lp.registry.model.distroseriesdifference import DistroSeriesDifference
from lp.registry.model.distroseriesparent import DistroSeriesParent
+from lp.services.log.logger import DevNullLogger
from lp.services.scripts.base import LaunchpadScript
from lp.soyuz.interfaces.publishing import active_publishing_status
@@ -195,18 +199,29 @@
store.execute("DROP TABLE IF EXISTS %s" % quote_identifier(table))
-def populate_distroseriesdiff(logger, derived_series, parent_series):
+def populate_distroseriesdiff(derived_series, parent_series, logger=None):
"""Compare `derived_distroseries` to parent, and register differences.
The differences are registered by creating `DistroSeriesDifference`
records, insofar as they do not yet exist.
"""
+ if logger is None:
+ logger = DevNullLogger()
temp_table = "temp_potentialdistroseriesdiff"
store = IStore(derived_series)
drop_table(store, temp_table)
- store.execute("CREATE TEMP TABLE %s AS %s" % (
- quote_identifier(temp_table),
+ quoted_temp_table = quote_identifier(temp_table)
+ store.execute("""
+ CREATE TEMP TABLE %s(
+ sourcepackagename INTEGER,
+ source_version debversion,
+ parent_source_version debversion)
+ ON COMMIT DROP
+ """ % (
+ quoted_temp_table))
+ store.execute("INSERT INTO %s %s" % (
+ quoted_temp_table,
compose_sql_find_differences(derived_series, parent_series)))
logger.info(
"Found %d potential difference(s).",
@@ -214,7 +229,31 @@
store.execute(
compose_sql_populate_distroseriesdiff(
derived_series, parent_series, temp_table))
- drop_table(store, temp_table)
+
+
+def update_distroseriesdiff(commit, distroseries, logger=None):
+ """Call `DistroSeriesDifference.update()` where appropriate.
+
+ The `DistroSeriesDifference` records we create don't have their
+ details filled out, such as base version or their diffs.
+
+ Only instances where the source package is published in both the
+ parent series and the derived series need to have this done.
+ """
+ if logger is None:
+ logger = DevNullLogger()
+ logger.info(
+ "Updating DSDs for %s.", distroseries.title)
+ store = IStore(distroseries)
+ dsd_ids = store.find(
+ DistroSeriesDifference.id,
+ DistroSeriesDifference.derived_series == distroseries,
+ DistroSeriesDifference.status ==
+ DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
+ DistroSeriesDifference.difference_type ==
+ DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
+ DistroSeriesDifference.base_version == None)
+ DSDUpdater(logger, store, commit, dsd_ids).run()
def find_derived_series():
@@ -334,7 +373,7 @@
self.logger.info(
"Looking for differences in %s with regards to %s.",
distroseries, parent)
- populate_distroseriesdiff(self.logger, distroseries, parent)
+ populate_distroseriesdiff(distroseries, parent, self.logger)
self.commit()
self.logger.info("Updating base_versions.")
self.update(distroseries)
@@ -353,7 +392,7 @@
relationships = self.getDistroSeries()
for child in relationships:
for parent in relationships[child]:
- self.logger.info(
+ self.logger.info(
"%s %s with a parent of %s %s", child.distribution.name,
child.name, parent.distribution.name, parent.name)
@@ -388,23 +427,5 @@
self.processDistroSeries(child, parent)
def update(self, distroseries):
- """Call `DistroSeriesDifference.update()` where appropriate.
-
- The `DistroSeriesDifference` records we create don't have their
- details filled out, such as base version or their diffs.
-
- Only instances where the source package is published in both the
- parent series and the derived series need to have this done.
- """
- self.logger.info(
- "Updating DSDs for %s.", distroseries.title)
- store = IStore(distroseries)
- dsd_ids = store.find(
- DistroSeriesDifference.id,
- DistroSeriesDifference.derived_series == distroseries,
- DistroSeriesDifference.status ==
- DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
- DistroSeriesDifference.difference_type ==
- DistroSeriesDifferenceType.DIFFERENT_VERSIONS,
- DistroSeriesDifference.base_version == None)
- DSDUpdater(self.logger, store, self.commit, dsd_ids).run()
+ """Update `DistroSeriesDifference`s for `distroseries`."""
+ update_distroseriesdiff(self.commit, distroseries, self.logger)
=== modified file 'lib/lp/registry/scripts/tests/test_populate_distroseriesdiff.py'
--- lib/lp/registry/scripts/tests/test_populate_distroseriesdiff.py 2011-05-23 10:54:01 +0000
+++ lib/lp/registry/scripts/tests/test_populate_distroseriesdiff.py 2011-07-19 12:56:28 +0000
@@ -24,11 +24,11 @@
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.model.distroseriesdifference import DistroSeriesDifference
from lp.registry.scripts.populate_distroseriesdiff import (
- DSDUpdater,
compose_sql_difference_type,
+ compose_sql_find_differences,
compose_sql_find_latest_source_package_releases,
- compose_sql_find_differences,
compose_sql_populate_distroseriesdiff,
+ DSDUpdater,
find_derived_series,
populate_distroseriesdiff,
PopulateDistroSeriesDiff,
@@ -37,12 +37,12 @@
BufferLogger,
DevNullLogger,
)
+from lp.soyuz.enums import ArchivePurpose
from lp.soyuz.interfaces.publishing import (
active_publishing_status,
inactive_publishing_status,
)
from lp.soyuz.model.archive import Archive
-from lp.soyuz.enums import ArchivePurpose
from lp.testing import (
TestCase,
TestCaseWithFactory,
@@ -444,8 +444,7 @@
def test_creates_distroseriesdifference(self):
dsp = self.makeDerivedDistroSeries()
spph = self.makeSPPH(distroseries=dsp.derived_series)
- populate_distroseriesdiff(
- DevNullLogger(), dsp.derived_series, dsp.parent_series)
+ populate_distroseriesdiff(dsp.derived_series, dsp.parent_series)
dsd = self.getDistroSeriesDiff(dsp.derived_series).one()
spr = spph.sourcepackagerelease
self.assertEqual(spr.sourcepackagename, dsd.source_package_name)
@@ -458,7 +457,7 @@
changelog = self.factory.makeChangelog(versions=['3.1', '3.141'])
parent_changelog = self.factory.makeChangelog(
versions=['3.1', '3.14'])
- transaction.commit() # Yay, librarian.
+ transaction.commit() # Yay, librarian.
existing_versions = {
'base': '3.1',
'parent': '3.14',
=== modified file 'lib/lp/soyuz/scripts/initialize_distroseries.py'
--- lib/lp/soyuz/scripts/initialize_distroseries.py 2011-07-16 19:39:42 +0000
+++ lib/lp/soyuz/scripts/initialize_distroseries.py 2011-07-19 12:56:28 +0000
@@ -210,6 +210,7 @@
self._copy_architectures()
self._copy_packages()
self._copy_packagesets()
+ self._create_dsds()
self._set_initialized()
transaction.commit()
@@ -240,6 +241,61 @@
for distroseriesparent in distroseriesparents:
distroseriesparent.initialized = True
+ def _has_same_parents_as_previous_series(self):
+ # Does this distroseries have the same parents as it's previous
+ # series? (note that the parent's order does not matter here)
+ dsp_set = getUtility(IDistroSeriesParentSet)
+ previous_series_parents = [
+ dsp.parent_series for dsp in dsp_set.getByDerivedSeries(
+ self.distroseries.previous_series)]
+ return set(previous_series_parents) == set(self.parents)
+
+ def _create_dsds(self):
+ if not self.first_derivation:
+ if (self._has_same_parents_as_previous_series() and
+ not self.packagesets):
+ # If the parents are the same as previous_series's
+ # parents and all the packagesets are being copied,
+ # then we simply copy the DSDs from previous_series
+ # for performance reasons.
+ self._copy_dsds_from_previous_series()
+ else:
+ # Either the parents have changed (compared to
+ # previous_series's parent) or a selection only of the
+ # packagesets is being copied so we have to recompute
+ # the DSDs.
+ self._compute_dsds()
+
+ def _copy_dsds_from_previous_series(self):
+ self._store.execute("""
+ INSERT INTO DistroSeriesDifference
+ (derived_series, source_package_name, package_diff,
+ status, difference_type, parent_package_diff,
+ source_version, parent_source_version,
+ base_version, parent_series)
+ SELECT
+ %s AS derived_series, source_package_name,
+ package_diff, status,
+ difference_type, parent_package_diff, source_version,
+ parent_source_version, base_version, parent_series
+ FROM DistroSeriesDifference AS dsd
+ WHERE dsd.derived_series = %s
+ """ % sqlvalues(
+ self.distroseries.id,
+ self.distroseries.previous_series.id))
+
+ def _compute_dsds(self):
+ # Avoid circular imports.
+ from lp.registry.scripts.populate_distroseriesdiff import (
+ populate_distroseriesdiff,
+ update_distroseriesdiff,
+ )
+ for parent in self.parents:
+ populate_distroseriesdiff(self.distroseries, parent)
+ transaction.commit()
+ update_distroseriesdiff(transaction.commit, self.distroseries)
+ transaction.commit()
+
def _copy_configuration(self):
self.distroseries.backports_not_automatic = any(
parent.backports_not_automatic
=== modified file 'lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py 2011-07-13 11:53:55 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py 2011-07-19 12:56:28 +0000
@@ -18,6 +18,9 @@
from canonical.launchpad.interfaces.lpstorm import IStore
from canonical.testing.layers import LaunchpadZopelessLayer
from lp.buildmaster.enums import BuildStatus
+from lp.registry.interfaces.distroseriesdifference import (
+ IDistroSeriesDifferenceSource,
+ )
from lp.registry.interfaces.distroseriesparent import IDistroSeriesParentSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.soyuz.enums import SourcePackageFormat
@@ -40,6 +43,7 @@
InitializeDistroSeries,
)
from lp.testing import TestCaseWithFactory
+from lp.testing.fakemethod import FakeMethod
class InitializationHelperTestCase(TestCaseWithFactory):
@@ -704,7 +708,7 @@
InitializationError, self._fullInitialize,
[self.parent1, self.parent2])
- def setUpSeriesWithPreviousSeries(self, parent, previous_parents=(),
+ def setUpSeriesWithPreviousSeries(self, previous_parents=(),
publish_in_distribution=True):
# Helper method to create a series within an initialized
# distribution (i.e. that has an initialized series) with a
@@ -730,10 +734,9 @@
# the previous_series' parents.
previous_parent1, unused = self.setupParent(packages={u'p1': u'1.2'})
previous_parent2, unused = self.setupParent(packages={u'p2': u'1.5'})
- parent, unused = self.setupParent()
child = self.setUpSeriesWithPreviousSeries(
- parent=parent,
previous_parents=[previous_parent1, previous_parent2])
+ parent, unused = self.setupParent()
self._fullInitialize([parent], child=child)
# The parent for the derived series is the distroseries given as
@@ -759,9 +762,7 @@
# parents of the previous series are used as parents.
previous_parent1, unused = self.setupParent(packages={u'p1': u'1.2'})
previous_parent2, unused = self.setupParent(packages={u'p2': u'1.5'})
- parent, unused = self.setupParent()
child = self.setUpSeriesWithPreviousSeries(
- parent=parent,
previous_parents=[previous_parent1, previous_parent2])
# Initialize from an empty list of parents.
self._fullInitialize([], child=child)
@@ -773,10 +774,8 @@
def test_derive_empty_parents_distribution_not_initialized(self):
# Initializing a series with an empty parent list if the series'
# distribution has no initialized series triggers an error.
- parent, unused = self.setupParent()
previous_parent1, unused = self.setupParent(packages={u'p1': u'1.2'})
child = self.setUpSeriesWithPreviousSeries(
- parent=parent,
previous_parents=[previous_parent1],
publish_in_distribution=False)
@@ -852,3 +851,121 @@
self.assertFalse(
InitializeDistroSeries._use_cloner(
target_archive, target_archive, distroseries))
+
+ def test__has_same_parents_as_previous_series_explicit(self):
+ # IDS._has_same_parents_as_previous_series returns True if the
+ # parents for the series to be initialized are the same as
+ # previous_series' parents.
+ prev_parent1, unused = self.setupParent(packages={u'p1': u'1.2'})
+ prev_parent2, unused = self.setupParent(packages={u'p2': u'1.5'})
+ child = self.setUpSeriesWithPreviousSeries(
+ previous_parents=[prev_parent1, prev_parent2])
+ # The same parents can be explicitely set.
+ ids = InitializeDistroSeries(
+ child, [prev_parent2.id, prev_parent1.id])
+
+ self.assertTrue(ids._has_same_parents_as_previous_series())
+
+ def test__has_same_parents_as_previous_series_implicit(self):
+ # IDS._has_same_parents_as_previous_series returns True if the
+ # parents for the series to be initialized are the same as
+ # previous_series' parents.
+ prev_parent1, unused = self.setupParent(packages={u'p1': u'1.2'})
+ prev_parent2, unused = self.setupParent(packages={u'p2': u'1.5'})
+ child = self.setUpSeriesWithPreviousSeries(
+ previous_parents=[prev_parent1, prev_parent2])
+ # If no parents are provided, the parents from previous_series
+ # will be used.
+ ids = InitializeDistroSeries(child)
+
+ self.assertTrue(ids._has_same_parents_as_previous_series())
+
+ def test_not__has_same_parents_as_previous_series(self):
+ # IDS._has_same_parents_as_previous_series returns False if the
+ # parents for the series to be initialized are *not* the same as
+ # previous_series' parents.
+ prev_parent1, unused = self.setupParent(packages={u'p1': u'1.2'})
+ prev_parent2, unused = self.setupParent(packages={u'p2': u'1.5'})
+ child = self.setUpSeriesWithPreviousSeries(
+ previous_parents=[prev_parent1, prev_parent2])
+ parent3 = self.factory.makeDistroSeries()
+ ids = InitializeDistroSeries(
+ child, [prev_parent2.id, prev_parent1.id, parent3.id])
+
+ self.assertFalse(ids._has_same_parents_as_previous_series())
+
+ def test_initialization_copy_dsds(self):
+ # Post-first initialization of a series with the same parents
+ # than those of the previous_series causes a copy of
+ # previous_series' DSDs.
+ prev_parent1, unused = self.setupParent(packages={u'p1': u'1.2'})
+ prev_parent2, unused = self.setupParent(packages={u'p2': u'1.5'})
+ child = self.setUpSeriesWithPreviousSeries(
+ previous_parents=[prev_parent1, prev_parent2])
+ self.factory.makeDistroSeriesDifference()
+ self.factory.makeDistroSeriesDifference(
+ derived_series=child.previous_series,
+ source_package_name_str=u'p1')
+ self.factory.makeDistroSeriesDifference(
+ derived_series=child.previous_series,
+ source_package_name_str=u'p2')
+ dsd_source = getUtility(IDistroSeriesDifferenceSource)
+ # No DSDs for the child yet.
+ self.assertEquals(0, dsd_source.getForDistroSeries(child).count())
+ self._fullInitialize([], child=child)
+
+ self.assertContentEqual(
+ [u'p1', u'p2'],
+ sorted(
+ [diff.source_package_name.name
+ for diff in dsd_source.getForDistroSeries(child)]))
+
+ def test_initialization_compute_dsds(self):
+ # Post-first initialization of a series with different parents
+ # than those of the previous_series triggers the computation
+ # of the DSDs (using populate_distroseriesdiff's methods).
+ prev_parent1, unused = self.setupParent(packages={u'p1': u'1.2'})
+ prev_parent2, unused = self.setupParent(packages={u'p2': u'1.5'})
+ child = self.setUpSeriesWithPreviousSeries(
+ previous_parents=[prev_parent1, prev_parent2])
+ parent3, unused = self.setupParent(packages={u'p3': u'2.5'})
+ from lp.registry.scripts import populate_distroseriesdiff as module
+ module.populate_distroseriesdiff = FakeMethod()
+ module.update_distroseriesdiff = FakeMethod()
+ self._fullInitialize(
+ [prev_parent1, prev_parent2, parent3], child=child)
+
+ # populate_distroseriesdiff has been called for each parent.
+ self.assertContentEqual(
+ [((child, prev_parent1), {}),
+ ((child, prev_parent2), {}),
+ ((child, parent3), {})],
+ module.populate_distroseriesdiff.calls)
+
+ # update_distroseriesdiff has been called once for the child series.
+ self.assertContentEqual(
+ [((transaction.commit, child), {})],
+ module.update_distroseriesdiff.calls)
+
+ def test_initialization_compute_dsds_specific_packagesets(self):
+ # Post-first initialization of a series with specific
+ # packagesets triggers the computation (as opposed to the copy)
+ # of the DSDs.
+ prev_parent1, unused = self.setupParent(packages={u'p1': u'1.2'})
+ prev_parent2, unused = self.setupParent(packages={u'p2': u'1.5'})
+ test1 = getUtility(IPackagesetSet).new(
+ u'test1', u'test 1 packageset', prev_parent1.owner,
+ distroseries=prev_parent1)
+ test1.addSources('p1')
+ child = self.setUpSeriesWithPreviousSeries(
+ previous_parents=[prev_parent1, prev_parent2])
+ parent3, unused = self.setupParent(packages={u'p3': u'2.5'})
+ from lp.registry.scripts import populate_distroseriesdiff as module
+ module.update_distroseriesdiff = FakeMethod()
+ ids = InitializeDistroSeries(
+ child, [], packagesets=(str(test1.id),))
+ ids._compute_dsds = FakeMethod()
+ ids.check()
+ ids.initialize()
+
+ self.assertEquals(1, ids._compute_dsds.call_count)
Follow ups
