launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29625
[Merge] ~cjwatson/launchpad:remove-populate-snap-build-job-store-revision into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:remove-populate-snap-build-job-store-revision into launchpad:master.
Commit message:
Complete migration to SnapBuild.store_upload_revision
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/436597
This migration started in 2021, so the backfill job completed long ago.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:remove-populate-snap-build-job-store-revision into launchpad:master.
diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index 9021834..d7a4fbb 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -2571,7 +2571,6 @@ public.revisionstatusreport = SELECT, DELETE
public.signedcodeofconduct = SELECT, UPDATE
public.snap = SELECT, UPDATE
public.snapbase = SELECT
-public.snapbuild = SELECT, UPDATE
public.snapfile = SELECT, DELETE
public.snappydistroseries = SELECT
public.snappyseries = SELECT
diff --git a/lib/lp/scripts/garbo.py b/lib/lp/scripts/garbo.py
index 770c5d2..e798587 100644
--- a/lib/lp/scripts/garbo.py
+++ b/lib/lp/scripts/garbo.py
@@ -34,7 +34,6 @@ from storm.expr import (
Except,
In,
Join,
- LeftJoin,
Max,
Min,
Or,
@@ -118,8 +117,8 @@ from lp.services.verification.model.logintoken import LoginToken
from lp.services.webapp.publisher import canonical_url
from lp.services.webhooks.interfaces import IWebhookJobSource
from lp.services.webhooks.model import WebhookJob
-from lp.snappy.model.snapbuild import SnapBuild, SnapFile
-from lp.snappy.model.snapbuildjob import SnapBuildJob, SnapBuildJobType
+from lp.snappy.model.snapbuild import SnapFile
+from lp.snappy.model.snapbuildjob import SnapBuildJobType
from lp.soyuz.enums import (
ArchivePublishingMethod,
ArchiveRepositoryFormat,
@@ -1997,44 +1996,6 @@ class ArchiveAuthTokenDeactivator(BulkPruner):
transaction.commit()
-class PopulateSnapBuildStoreRevision(TunableLoop):
- """Populates snapbuild.store_revision if not set."""
-
- maximum_chunk_size = 5000
-
- def __init__(self, log, abort_time=None):
- super().__init__(log, abort_time)
- self.start_at = 1
- self.store = IPrimaryStore(SnapBuild)
-
- def findSnapBuilds(self):
- origin = [
- SnapBuild,
- LeftJoin(SnapBuildJob, SnapBuildJob.snapbuild_id == SnapBuild.id),
- LeftJoin(Job, Job.id == SnapBuildJob.job_id),
- ]
- builds = self.store.using(*origin).find(
- SnapBuild,
- SnapBuild.id >= self.start_at,
- SnapBuild._store_upload_revision == None,
- SnapBuildJob.job_type == SnapBuildJobType.STORE_UPLOAD,
- Job._status == JobStatus.COMPLETED,
- )
-
- return builds.order_by(SnapBuild.id)
-
- def isDone(self):
- return self.findSnapBuilds().is_empty()
-
- def __call__(self, chunk_size):
- builds = list(self.findSnapBuilds()[:chunk_size])
- for build in builds:
- build._store_upload_revision = build.store_upload_revision
- if len(builds):
- self.start_at = builds[-1].id + 1
- transaction.commit()
-
-
class RevisionStatusReportPruner(BulkPruner):
"""Removes old revision status reports and their artifacts."""
@@ -2610,7 +2571,6 @@ class DailyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
OCIFilePruner,
ObsoleteBugAttachmentPruner,
OldTimeLimitedTokenDeleter,
- PopulateSnapBuildStoreRevision,
POTranslationPruner,
PreviewDiffPruner,
ProductVCSPopulator,
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index ade0b76..1d22660 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -80,7 +80,6 @@ from lp.scripts.garbo import (
HourlyDatabaseGarbageCollector,
LoginTokenPruner,
OpenIDConsumerAssociationPruner,
- PopulateSnapBuildStoreRevision,
ProductVCSPopulator,
UnusedPOTMsgSetPruner,
UnusedSessionPruner,
@@ -112,14 +111,8 @@ from lp.services.verification.interfaces.authtoken import LoginTokenType
from lp.services.verification.model.logintoken import LoginToken
from lp.services.worlddata.interfaces.language import ILanguageSet
from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS
-from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
-from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
from lp.snappy.model.snapbuild import SnapFile
from lp.snappy.model.snapbuildjob import SnapBuildJob, SnapStoreUploadJob
-from lp.snappy.tests.test_snapbuildjob import (
- FakeSnapStoreClient,
- run_isolated_jobs,
-)
from lp.soyuz.enums import (
ArchivePublishingMethod,
ArchiveRepositoryFormat,
@@ -143,8 +136,7 @@ from lp.testing import (
admin_logged_in,
person_logged_in,
)
-from lp.testing.dbuser import dbuser, switch_dbuser
-from lp.testing.fixture import ZopeUtilityFixture
+from lp.testing.dbuser import switch_dbuser
from lp.testing.layers import (
DatabaseLayer,
LaunchpadScriptLayer,
@@ -2248,67 +2240,6 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
self.assertIsNotNone(token.date_deactivated)
self.assertEmailQueueLength(0)
- def test_PopulateSnapBuildStoreRevision(self):
- switch_dbuser("testadmin")
- snap1 = self.factory.makeSnap()
- build1 = self.factory.makeSnapBuild(
- snap=snap1, status=BuildStatus.FULLYBUILT
- )
- snap1_lfa = self.factory.makeLibraryFileAlias(
- filename="test-snap.snap", content=b"dummy snap content"
- )
- self.factory.makeSnapFile(snapbuild=build1, libraryfile=snap1_lfa)
-
- # test that build1 does not get picked up
- # as it is a build without a store upload
- populator = PopulateSnapBuildStoreRevision(None)
- rs = populator.findSnapBuilds()
- self.assertEqual(0, rs.count())
-
- # Upload build
- job = getUtility(ISnapStoreUploadJobSource).create(build1)
- client = FakeSnapStoreClient()
- client.uploadFile.result = 1
- client.push.result = (
- "http://sca.example/dev/api/snaps/1/builds/1/status";
- )
- client.checkStatus.result = (
- "http://sca.example/dev/click-apps/1/rev/1/";,
- 1,
- )
- self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
- with dbuser(config.ISnapStoreUploadJobSource.dbuser):
- run_isolated_jobs([job])
-
- # The _store_upload_revision now gets populated when
- # uploading the build as part of the
- # `SnapStoreUploadJob.store_revision` property setter.
- # Assert that upload job above populated _store_upload_revision
- # for build1:
- build1 = removeSecurityProxy(build1)
- self.assertEqual(build1._store_upload_revision, 1)
-
- # and that the populator job doesn't pick up build1 anymore.
- populator = PopulateSnapBuildStoreRevision(None)
- filter = populator.findSnapBuilds()
- self.assertEqual(0, filter.count())
-
- # We manually simulate here the situation
- # where a job to upload snap to the store has run in the past
- # without populating the new column (prior to MP 407781).
- build1._store_upload_revision = None
-
- # in this case the populator garbo job should pick up build1
- # to update the _store_upload_revision for it:
- populator = PopulateSnapBuildStoreRevision(None)
- filter = populator.findSnapBuilds()
- self.assertEqual(1, filter.count())
- self.assertEqual(build1, filter.one())
-
- self.runDaily()
- switch_dbuser("testadmin")
- self.assertEqual(build1._store_upload_revision, 1)
-
def test_RevisionStatusReportPruner(self):
# Artifacts for report1 are older than 90 days
# and we expect the garbo job to delete them.
diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index 35c54d3..6bbcf8f 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -1252,7 +1252,7 @@ class Snap(Storm, WebhookTargetMixin):
.find(
SnapBuild,
SnapBuild.snap == self,
- SnapBuild._store_upload_revision == store_upload_revision,
+ SnapBuild.store_upload_revision == store_upload_revision,
SnapBuild.archive == Archive.id,
Archive._enabled,
get_enabled_archive_filter(
diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
index 18217e3..2ae196c 100644
--- a/lib/lp/snappy/model/snapbuild.py
+++ b/lib/lp/snappy/model/snapbuild.py
@@ -181,7 +181,7 @@ class SnapBuild(PackageBuildMixin, Storm):
failure_count = Int(name="failure_count", allow_none=False)
- _store_upload_revision = Int(name="store_upload_revision", allow_none=True)
+ store_upload_revision = Int(name="store_upload_revision", allow_none=True)
store_upload_metadata = JSON("store_upload_json_data", allow_none=True)
@@ -500,21 +500,6 @@ class SnapBuild(PackageBuildMixin, Storm):
return job and job.store_url
@property
- def store_upload_revision(self):
- # We are now persisting the revision assigned by the store
- # on package upload in the new DB column _store_upload_revision.
- # We backfill _store_upload_revision with
- # PopulateSnapBuildStoreRevision.
- # If the persisted field (_store_upload_revision)
- # is not populated yet we return the old way of computing this
- # value so that we don't break existing API clients.
- if self._store_upload_revision:
- return self._store_upload_revision
- else:
- job = self.last_store_upload_job
- return job and job.store_revision
-
- @property
def store_upload_error_message(self):
job = self.last_store_upload_job
return job and job.error_message
diff --git a/lib/lp/snappy/model/snapbuildjob.py b/lib/lp/snappy/model/snapbuildjob.py
index 7ebd3f0..b16d2c8 100644
--- a/lib/lp/snappy/model/snapbuildjob.py
+++ b/lib/lp/snappy/model/snapbuildjob.py
@@ -276,7 +276,7 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
if self.snapbuild.store_upload_metadata is None:
self.snapbuild.store_upload_metadata = {}
self.snapbuild.store_upload_metadata["store_revision"] = revision
- self.snapbuild._store_upload_revision = revision
+ self.snapbuild.store_upload_revision = revision
# Ideally we'd just override Job._set_status or similar, but
# lazr.delegates makes that difficult, so we use this to override all
diff --git a/lib/lp/snappy/tests/test_snapbuildjob.py b/lib/lp/snappy/tests/test_snapbuildjob.py
index 9c2ad2a..e8855cc 100644
--- a/lib/lp/snappy/tests/test_snapbuildjob.py
+++ b/lib/lp/snappy/tests/test_snapbuildjob.py
@@ -221,9 +221,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertEqual(1, job.store_revision)
- self.assertEqual(
- 1, removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertEqual(1, snapbuild.store_upload_revision)
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
self.assertWebhookDeliveries(
@@ -249,9 +247,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
- self.assertIsNone(
- removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertIsNone(snapbuild.store_upload_revision)
self.assertEqual("An upload failure", job.error_message)
self.assertEqual([], pop_notifications())
self.assertWebhookDeliveries(
@@ -286,9 +282,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
- self.assertIsNone(
- removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertIsNone(snapbuild.store_upload_revision)
self.assertEqual("Authorization failed.", job.error_message)
[notification] = pop_notifications()
self.assertEqual(
@@ -350,9 +344,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
- self.assertIsNone(
- removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertIsNone(snapbuild.store_upload_revision)
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
self.assertEqual(JobStatus.WAITING, job.job.status)
@@ -375,9 +367,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertEqual(1, job.store_revision)
- self.assertEqual(
- 1, removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertEqual(1, snapbuild.store_upload_revision)
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
self.assertEqual(JobStatus.COMPLETED, job.job.status)
@@ -412,9 +402,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
- self.assertIsNone(
- removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertIsNone(snapbuild.store_upload_revision)
self.assertEqual("SSO melted.", job.error_message)
[notification] = pop_notifications()
self.assertEqual(
@@ -484,9 +472,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
- self.assertIsNone(
- removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertIsNone(snapbuild.store_upload_revision)
self.assertEqual("Failed to upload", job.error_message)
[notification] = pop_notifications()
self.assertEqual(
@@ -552,9 +538,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
- self.assertIsNone(
- removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertIsNone(snapbuild.store_upload_revision)
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
self.assertEqual(JobStatus.WAITING, job.job.status)
@@ -575,9 +559,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertEqual(1, job.store_revision)
- self.assertEqual(
- 1, removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertEqual(1, snapbuild.store_upload_revision)
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
self.assertEqual(JobStatus.COMPLETED, job.job.status)
@@ -618,9 +600,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
- self.assertIsNone(
- removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertIsNone(snapbuild.store_upload_revision)
self.assertEqual(
"Scan failed.\nConfinement not allowed.", job.error_message
)
@@ -689,9 +669,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
- self.assertIsNone(
- removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertIsNone(snapbuild.store_upload_revision)
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
self.assertWebhookDeliveries(
@@ -720,9 +698,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertEqual(1, job.store_revision)
- self.assertEqual(
- 1, removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertEqual(1, snapbuild.store_upload_revision)
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
self.assertWebhookDeliveries(
@@ -797,9 +773,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
# We performed the upload as expected, but the push failed.
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
- self.assertIsNone(
- removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertIsNone(snapbuild.store_upload_revision)
self.assertEqual(timedelta(seconds=60), job.retry_delay)
self.assertEqual(1, len(client.uploadFile.calls))
self.assertIsNone(job.error_message)
@@ -840,9 +814,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
# still scanning it.
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
- self.assertIsNone(
- removeSecurityProxy(snapbuild)._store_upload_revision
- )
+ self.assertIsNone(snapbuild.store_upload_revision)
self.assertEqual(timedelta(seconds=15), job.retry_delay)
self.assertEqual(1, len(client.uploadFile.calls))
self.assertEqual(1, len(client.push.calls))
