launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #23250
[Merge] lp:~cjwatson/launchpad/remove-snap-store-series-backfill into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/remove-snap-store-series-backfill into lp:launchpad.
Commit message:
Remove migration code for snaps without store_series.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/remove-snap-store-series-backfill/+merge/362459
The only remaining rows on production are one snap that inexplicably sets precise as its distro_series (which will never have a corresponding store_series) and sometimes some snaps created by build.snapcraft.io (fixed by https://github.com/canonical-websites/build.snapcraft.io/pull/1190, but harmless since even before that it sets store_series when authorising the snap store upload). As such, I think it's OK to simplify things a bit and remove this data migration and its associated support code.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-snap-store-series-backfill into lp:launchpad.
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2018-12-17 14:49:53 +0000
+++ lib/lp/scripts/garbo.py 2019-01-30 14:07:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Database garbage collection."""
@@ -12,7 +12,6 @@
'save_garbo_job_state',
]
-from collections import defaultdict
from datetime import (
datetime,
timedelta,
@@ -121,8 +120,6 @@
from lp.services.verification.model.logintoken import LoginToken
from lp.services.webhooks.interfaces import IWebhookJobSource
from lp.services.webhooks.model import WebhookJob
-from lp.snappy.interfaces.snappyseries import ISnappyDistroSeriesSet
-from lp.snappy.model.snap import Snap
from lp.snappy.model.snapbuild import SnapFile
from lp.snappy.model.snapbuildjob import SnapBuildJobType
from lp.soyuz.enums import PackagePublishingStatus
@@ -1614,45 +1611,6 @@
""" % (SnapBuildJobType.STORE_UPLOAD.value, JobStatus.COMPLETED.value)
-class SnapStoreSeriesPopulator(TunableLoop):
- """Populates Snap.store_series based on Snap.distro_series.
-
- This only touches rows where there is exactly one SnappySeries that
- could be built from the relevant DistroSeries.
- """
-
- maximum_chunk_size = 5000
-
- def __init__(self, log, abort_time=None):
- super(SnapStoreSeriesPopulator, self).__init__(log, abort_time)
- self.start_at = 1
- self.store = IMasterStore(Snap)
- all_series_map = defaultdict(list)
- for sds in getUtility(ISnappyDistroSeriesSet).getAll():
- all_series_map[sds.distro_series.id].append(sds.snappy_series)
- self.series_map = {
- distro_series_id: snappy_serieses[0]
- for distro_series_id, snappy_serieses in all_series_map.items()
- if len(snappy_serieses) == 1}
-
- def findSnaps(self):
- return self.store.find(
- Snap,
- Snap.id >= self.start_at,
- Snap.distro_series_id.is_in(self.series_map),
- Snap.store_series == None).order_by(Snap.id)
-
- def isDone(self):
- return self.findSnaps().is_empty()
-
- def __call__(self, chunk_size):
- snaps = list(self.findSnaps()[:chunk_size])
- for snap in snaps:
- snap.store_series = self.series_map[snap.distro_series_id]
- self.start_at = snaps[-1].id + 1
- transaction.commit()
-
-
class BaseDatabaseGarbageCollector(LaunchpadCronScript):
"""Abstract base class to run a collection of TunableLoops."""
script_name = None # Script name for locking and database user. Override.
@@ -1945,7 +1903,6 @@
ScrubPOFileTranslator,
SnapBuildJobPruner,
SnapFilePruner,
- SnapStoreSeriesPopulator,
SuggestiveTemplatesCacheUpdater,
TeamMembershipPruner,
UnlinkedAccountPruner,
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py 2018-12-17 14:49:53 +0000
+++ lib/lp/scripts/tests/test_garbo.py 2019-01-30 14:07:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test the database garbage collector."""
@@ -1606,41 +1606,6 @@
# retained.
self._test_SnapFilePruner('foo.snap', None, 30, expected_count=1)
- def test_SnapStoreSeriesPopulator(self):
- switch_dbuser('testadmin')
- # Make some series.
- dses = [self.factory.makeDistroSeries() for _ in range(4)]
- sses = [
- self.factory.makeSnappySeries(usable_distro_series=[dses[1]]),
- self.factory.makeSnappySeries(usable_distro_series=[dses[2]]),
- self.factory.makeSnappySeries(usable_distro_series=[dses[3]]),
- self.factory.makeSnappySeries(usable_distro_series=[dses[3]]),
- ]
- # Make some snap packages.
- snaps = [
- self.factory.makeSnap(distroseries=dses[0]),
- self.factory.makeSnap(distroseries=dses[1], store_series=sses[1]),
- self.factory.makeSnap(distroseries=dses[1]),
- self.factory.makeSnap(distroseries=dses[2], store_series=sses[0]),
- self.factory.makeSnap(distroseries=dses[2]),
- self.factory.makeSnap(distroseries=dses[3]),
- ]
- transaction.commit()
-
- self.runDaily()
-
- # Snaps with no possible store series are untouched.
- self.assertIsNone(snaps[0].store_series)
- # Snaps that already have a store series are untouched.
- self.assertEqual(sses[1], snaps[1].store_series)
- self.assertEqual(sses[0], snaps[3].store_series)
- # Snaps with no current store series and exactly one possible store
- # series have it filled in.
- self.assertEqual(sses[0], snaps[2].store_series)
- self.assertEqual(sses[1], snaps[4].store_series)
- # Snaps with more than one possible store series are untouched.
- self.assertIsNone(snaps[5].store_series)
-
class TestGarboTasks(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py 2018-10-09 09:25:19 +0000
+++ lib/lp/snappy/browser/snap.py 2019-01-30 14:07:49 +0000
@@ -750,12 +750,6 @@
@property
def initial_values(self):
initial_values = {}
- if self.context.store_series is None:
- # XXX cjwatson 2016-04-26: Remove this case once all existing
- # Snaps have had a store_series backfilled.
- sds_set = getUtility(ISnappyDistroSeriesSet)
- initial_values['store_distro_series'] = sds_set.getByDistroSeries(
- self.context.distro_series).first()
if self.context.git_ref is not None:
initial_values['vcs'] = VCSType.GIT
else:
=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py 2018-09-27 13:51:42 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py 2019-01-30 14:07:49 +0000
@@ -667,27 +667,6 @@
self.snappyseries = self.factory.makeSnappySeries(
usable_distro_series=[self.distroseries])
- def test_initial_store_series(self):
- # The initial store_series is the newest that is usable for the
- # selected distroseries.
- development = self.factory.makeUbuntuDistroSeries(
- version="14.10", status=SeriesStatus.DEVELOPMENT)
- experimental = self.factory.makeUbuntuDistroSeries(
- version="15.04", status=SeriesStatus.EXPERIMENTAL)
- with admin_logged_in():
- self.factory.makeSnappySeries(
- usable_distro_series=[development, experimental])
- newest = self.factory.makeSnappySeries(
- usable_distro_series=[development])
- self.factory.makeSnappySeries(usable_distro_series=[experimental])
- snap = self.factory.makeSnap(distroseries=development)
- with person_logged_in(self.person):
- view = create_initialized_view(snap, "+edit")
- self.assertThat(
- view.initial_values["store_distro_series"],
- MatchesStructure.byEquality(
- snappy_series=newest, distro_series=development))
-
def test_edit_snap(self):
old_series = self.factory.makeUbuntuDistroSeries()
old_branch = self.factory.makeAnyBranch()
@@ -808,8 +787,8 @@
"Source:\n%s\nEdit snap package" % new_ref.display_name,
MatchesTagText(content, "source"))
- def setUpDistroSeries(self):
- """Set up a distroseries with some available processors."""
+ def setUpSeries(self):
+ """Set up {distro,snappy}series with some available processors."""
distroseries = self.factory.makeUbuntuDistroSeries()
processor_names = ["386", "amd64", "hppa"]
for name in processor_names:
@@ -818,8 +797,9 @@
distroseries=distroseries, architecturetag=name,
processor=processor)
with admin_logged_in():
- self.factory.makeSnappySeries(usable_distro_series=[distroseries])
- return distroseries
+ snappyseries = self.factory.makeSnappySeries(
+ usable_distro_series=[distroseries])
+ return distroseries, snappyseries
def assertSnapProcessors(self, snap, names):
self.assertContentEqual(
@@ -835,10 +815,10 @@
self.assertThat(processors_control.controls, MatchesSetwise(*matchers))
def test_display_processors(self):
- distroseries = self.setUpDistroSeries()
+ distroseries, snappyseries = self.setUpSeries()
snap = self.factory.makeSnap(
registrant=self.person, owner=self.person,
- distroseries=distroseries)
+ distroseries=distroseries, store_series=snappyseries)
browser = self.getViewBrowser(snap, view_name="+edit", user=snap.owner)
processors = browser.getControl(name="field.processors")
self.assertContentEqual(
@@ -847,10 +827,10 @@
self.assertContentEqual(["386", "amd64", "hppa"], processors.options)
def test_edit_processors(self):
- distroseries = self.setUpDistroSeries()
+ distroseries, snappyseries = self.setUpSeries()
snap = self.factory.makeSnap(
registrant=self.person, owner=self.person,
- distroseries=distroseries)
+ distroseries=distroseries, store_series=snappyseries)
self.assertSnapProcessors(snap, ["386", "amd64", "hppa"])
browser = self.getViewBrowser(snap, view_name="+edit", user=snap.owner)
processors = browser.getControl(name="field.processors")
@@ -871,10 +851,10 @@
proc_amd64 = getUtility(IProcessorSet).getByName("amd64")
proc_armel = self.factory.makeProcessor(
name="armel", restricted=True, build_by_default=False)
- distroseries = self.setUpDistroSeries()
+ distroseries, snappyseries = self.setUpSeries()
snap = self.factory.makeSnap(
registrant=self.person, owner=self.person,
- distroseries=distroseries)
+ distroseries=distroseries, store_series=snappyseries)
snap.setProcessors([proc_386, proc_amd64, proc_armel])
browser = self.getViewBrowser(snap, view_name="+edit", user=snap.owner)
processors = browser.getControl(name="field.processors")
@@ -887,7 +867,7 @@
def test_edit_processors_restricted(self):
# A restricted processor is shown disabled in the UI and cannot be
# enabled.
- distroseries = self.setUpDistroSeries()
+ distroseries, snappyseries = self.setUpSeries()
proc_armhf = self.factory.makeProcessor(
name="armhf", restricted=True, build_by_default=False)
self.factory.makeDistroArchSeries(
@@ -895,7 +875,7 @@
processor=proc_armhf)
snap = self.factory.makeSnap(
registrant=self.person, owner=self.person,
- distroseries=distroseries)
+ distroseries=distroseries, store_series=snappyseries)
self.assertSnapProcessors(snap, ["386", "amd64", "hppa"])
browser = self.getViewBrowser(snap, view_name="+edit", user=snap.owner)
processors = browser.getControl(name="field.processors")
@@ -921,13 +901,13 @@
proc_amd64 = getUtility(IProcessorSet).getByName("amd64")
proc_armhf = self.factory.makeProcessor(
name="armhf", restricted=True, build_by_default=False)
- distroseries = self.setUpDistroSeries()
+ distroseries, snappyseries = self.setUpSeries()
self.factory.makeDistroArchSeries(
distroseries=distroseries, architecturetag="armhf",
processor=proc_armhf)
snap = self.factory.makeSnap(
registrant=self.person, owner=self.person,
- distroseries=distroseries)
+ distroseries=distroseries, store_series=snappyseries)
snap.setProcessors([proc_386, proc_amd64, proc_armhf])
self.assertSnapProcessors(snap, ["386", "amd64", "armhf"])
browser = self.getUserBrowser(
=== modified file 'lib/lp/snappy/interfaces/snappyseries.py'
--- lib/lp/snappy/interfaces/snappyseries.py 2016-11-08 16:51:34 +0000
+++ lib/lp/snappy/interfaces/snappyseries.py 2019-01-30 14:07:49 +0000
@@ -179,15 +179,6 @@
:raises NoSuchSnappySeries: if no snappy series exists with this name.
"""
- @operation_parameters(
- distro_series=Reference(
- IDistroSeries, title=_("Distro series"), required=True))
- @operation_returns_collection_of(ISnappySeries)
- @export_read_operation()
- @operation_for_version("devel")
- def getByDistroSeries(distro_series):
- """Return all `ISnappySeries` usable with this `IDistroSeries`."""
-
@collection_default_content()
def getAll():
"""Return all `ISnappySeries`."""
@@ -196,9 +187,6 @@
class ISnappyDistroSeriesSet(Interface):
"""Interface representing the set of snappy/distro series links."""
- def getByDistroSeries(distro_series):
- """Return all `SnappyDistroSeries` for this `IDistroSeries`."""
-
def getByBothSeries(snappy_series, distro_series):
"""Return a `SnappyDistroSeries` for this pair of series, or None."""
=== modified file 'lib/lp/snappy/model/snappyseries.py'
--- lib/lp/snappy/model/snappyseries.py 2016-11-15 14:11:16 +0000
+++ lib/lp/snappy/model/snappyseries.py 2019-01-30 14:07:49 +0000
@@ -197,14 +197,6 @@
raise NoSuchSnappySeries(name)
return snappy_series
- def getByDistroSeries(self, distro_series):
- """See `ISnappySeriesSet`."""
- rows = IStore(SnappySeries).find(
- SnappySeries,
- SnappyDistroSeries.snappy_series_id == SnappySeries.id,
- SnappyDistroSeries.distro_series == distro_series)
- return rows.order_by(Desc(SnappySeries.name))
-
def getAll(self):
"""See `ISnappySeriesSet`."""
return IStore(SnappySeries).find(SnappySeries).order_by(
@@ -215,15 +207,6 @@
class SnappyDistroSeriesSet:
"""See `ISnappyDistroSeriesSet`."""
- def getByDistroSeries(self, distro_series):
- """See `ISnappyDistroSeriesSet`."""
- store = IStore(SnappyDistroSeries)
- rows = store.using(SnappyDistroSeries, SnappySeries).find(
- SnappyDistroSeries,
- SnappyDistroSeries.snappy_series_id == SnappySeries.id,
- SnappyDistroSeries.distro_series == distro_series)
- return rows.order_by(Desc(SnappySeries.name))
-
def getByBothSeries(self, snappy_series, distro_series):
"""See `ISnappyDistroSeriesSet`."""
return IStore(SnappyDistroSeries).find(
=== modified file 'lib/lp/snappy/tests/test_snappyseries.py'
--- lib/lp/snappy/tests/test_snappyseries.py 2016-11-15 14:11:16 +0000
+++ lib/lp/snappy/tests/test_snappyseries.py 2019-01-30 14:07:49 +0000
@@ -112,22 +112,6 @@
self.assertRaises(
NoSuchSnappySeries, snappy_series_set.getByName, "bar")
- def test_getByDistroSeries(self):
- dses = [self.factory.makeDistroSeries() for _ in range(3)]
- snappy_serieses = [self.factory.makeSnappySeries() for _ in range(3)]
- snappy_serieses[0].usable_distro_series = dses
- snappy_serieses[1].usable_distro_series = [dses[0], dses[1]]
- snappy_serieses[2].usable_distro_series = [dses[1], dses[2]]
- snappy_series_set = getUtility(ISnappySeriesSet)
- self.assertContentEqual(
- [snappy_serieses[0], snappy_serieses[1]],
- snappy_series_set.getByDistroSeries(dses[0]))
- self.assertContentEqual(
- snappy_serieses, snappy_series_set.getByDistroSeries(dses[1]))
- self.assertContentEqual(
- [snappy_serieses[0], snappy_serieses[2]],
- snappy_series_set.getByDistroSeries(dses[2]))
-
def test_getAll(self):
sample_snappy_serieses = list(IStore(SnappySeries).find(SnappySeries))
snappy_serieses = [self.factory.makeSnappySeries() for _ in range(3)]
@@ -222,33 +206,6 @@
self.assertEqual(
"No such snappy series: 'nonexistent'.", response.body)
- def test_getByDistroSeries(self):
- # lp.snappy_serieses.getByDistroSeries returns a collection of
- # matching SnappySeries.
- person = self.factory.makePerson()
- webservice = webservice_for_person(
- person, permission=OAuthPermission.READ_PUBLIC)
- webservice.default_api_version = "devel"
- with admin_logged_in():
- dses = [self.factory.makeDistroSeries() for _ in range(3)]
- ds_urls = [api_url(ds) for ds in dses]
- snappy_serieses = [
- self.factory.makeSnappySeries(name="ss-%d" % i)
- for i in range(3)]
- snappy_serieses[0].usable_distro_series = dses
- snappy_serieses[1].usable_distro_series = [dses[0], dses[1]]
- snappy_serieses[2].usable_distro_series = [dses[1], dses[2]]
- for ds_url, expected_snappy_series_names in (
- (ds_urls[0], ["ss-0", "ss-1"]),
- (ds_urls[1], ["ss-0", "ss-1", "ss-2"]),
- (ds_urls[2], ["ss-0", "ss-2"])):
- response = webservice.named_get(
- "/+snappy-series", "getByDistroSeries", distro_series=ds_url)
- self.assertEqual(200, response.status)
- self.assertContentEqual(
- expected_snappy_series_names,
- [entry["name"] for entry in response.jsonBody()["entries"]])
-
def test_collection(self):
# lp.snappy_serieses is a collection of all SnappySeries.
person = self.factory.makePerson()
@@ -273,32 +230,6 @@
super(TestSnappyDistroSeriesSet, self).setUp()
self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
- def test_getByDistroSeries(self):
- dses = [self.factory.makeDistroSeries() for _ in range(3)]
- snappy_serieses = [self.factory.makeSnappySeries() for _ in range(3)]
- snappy_serieses[0].usable_distro_series = dses
- snappy_serieses[1].usable_distro_series = [dses[0], dses[1]]
- snappy_serieses[2].usable_distro_series = [dses[1], dses[2]]
- sds_set = getUtility(ISnappyDistroSeriesSet)
- self.assertThat(
- sds_set.getByDistroSeries(dses[0]),
- MatchesSetwise(*(
- MatchesStructure.byEquality(
- snappy_series=ss, distro_series=dses[0])
- for ss in (snappy_serieses[0], snappy_serieses[1]))))
- self.assertThat(
- sds_set.getByDistroSeries(dses[1]),
- MatchesSetwise(*(
- MatchesStructure.byEquality(
- snappy_series=ss, distro_series=dses[1])
- for ss in snappy_serieses)))
- self.assertThat(
- sds_set.getByDistroSeries(dses[2]),
- MatchesSetwise(*(
- MatchesStructure.byEquality(
- snappy_series=ss, distro_series=dses[2])
- for ss in (snappy_serieses[0], snappy_serieses[2]))))
-
def test_getByBothSeries(self):
dses = [self.factory.makeDistroSeries() for _ in range(2)]
snappy_serieses = [self.factory.makeSnappySeries() for _ in range(2)]
Follow ups
