launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30975
[Merge] ~pelpsi/launchpad:snap-component-integration into launchpad:master
Simone Pelosi has proposed merging ~pelpsi/launchpad:snap-component-integration into launchpad:master.
Commit message:
Add Snap Component support
Reference: LP142
Snap Components are processed and uploaded to the
snapcraft storage before pushing the Snap to the store.
Once every component is updated on the storage we can push
the Snap to the store.
push` function changed to support the new `components` parameter.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/461063
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad:snap-component-integration into launchpad:master.
diff --git a/lib/lp/snappy/interfaces/snapbuildjob.py b/lib/lp/snappy/interfaces/snapbuildjob.py
index 12e4b8b..c1bfc24 100644
--- a/lib/lp/snappy/interfaces/snapbuildjob.py
+++ b/lib/lp/snappy/interfaces/snapbuildjob.py
@@ -13,7 +13,7 @@ __all__ = [
from lazr.restful.fields import Reference
from zope.interface import Attribute, Interface
from zope.interface.interfaces import IObjectEvent
-from zope.schema import Int, TextLine
+from zope.schema import Dict, Int, TextLine
from lp import _
from lp.services.job.interfaces.job import IJob, IJobSource, IRunnableJob
@@ -68,6 +68,15 @@ class ISnapStoreUploadJob(IRunnableJob):
readonly=True,
)
+ components_ids = Dict(
+ title=_(
+ "The IDs returned by the store when uploading snap components."
+ "The key is the component name and the value is the related id."
+ ),
+ required=False,
+ readonly=True,
+ )
+
status_url = TextLine(
title=_("The URL on the store to get the status of this build"),
required=False,
diff --git a/lib/lp/snappy/model/snapbuildjob.py b/lib/lp/snappy/model/snapbuildjob.py
index 539fb1c..0384b0c 100644
--- a/lib/lp/snappy/model/snapbuildjob.py
+++ b/lib/lp/snappy/model/snapbuildjob.py
@@ -244,6 +244,18 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
self.snapbuild.store_upload_metadata["upload_id"] = upload_id
@property
+ def components_ids(self):
+ """See `ISnapStoreUploadJob`."""
+ return self.store_metadata.get("components_ids")
+
+ @components_ids.setter
+ def components_ids(self, components_ids):
+ """See `ISnapStoreUploadJob`."""
+ if self.snapbuild.store_upload_metadata is None:
+ self.snapbuild.store_upload_metadata = {}
+ self.snapbuild.store_upload_metadata["components_ids"] = components_ids
+
+ @property
def status_url(self):
"""See `ISnapStoreUploadJob`."""
return self.store_metadata.get("status_url")
@@ -333,6 +345,15 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
pass
return timedelta(minutes=1)
+ def _extract_component_name(self, filename):
+ # <snap name>+<component name>_<component revision>.comp
+ start = filename.find("+")
+ end = filename.rfind("_")
+ if start == -1 or end == -1:
+ return filename
+ else:
+ return filename[start + 1 : end]
+
def run(self):
"""See `IRunnableJob`."""
client = getUtility(ISnapStoreClient)
@@ -346,6 +367,17 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
),
None,
)
+ # Get components if any
+ components = []
+ components_ids = {}
+ for _, lfa, _ in self.snapbuild.getFiles():
+ if lfa.filename.endswith(".comp"):
+ comp_name = self._extract_component_name(lfa.filename)
+ components_ids[comp_name] = None
+ components.append(lfa)
+
+ if "components_ids" not in self.store_metadata:
+ self.components_ids = components_ids
if snap_lfa is None:
# Nothing to do.
self.error_message = None
@@ -354,12 +386,24 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
self.upload_id = client.uploadFile(snap_lfa)
# We made progress, so reset attempt_count.
self.attempt_count = 1
+ # Process components
+ for component in components:
+ # if the id is None, we need to upload the component
+ comp_name = self._extract_component_name(
+ component.filename
+ )
+ if self.components_ids.get(comp_name) == None:
+ self.components_ids[comp_name] = client.uploadFile(
+ component
+ )
+ self.attempt_count = 1
if "status_url" not in self.store_metadata:
self.status_url = client.push(
- self.snapbuild, self.upload_id
+ self.snapbuild, self.upload_id, self.components_ids
)
# We made progress, so reset attempt_count.
self.attempt_count = 1
+
if self.store_url is None:
# This is no longer strictly necessary as the store is
# handling releases via the release intent, but we
diff --git a/lib/lp/snappy/model/snapstoreclient.py b/lib/lp/snappy/model/snapstoreclient.py
index 536ba17..b3a4632 100644
--- a/lib/lp/snappy/model/snapstoreclient.py
+++ b/lib/lp/snappy/model/snapstoreclient.py
@@ -293,7 +293,7 @@ class SnapStoreClient:
lfa.close()
@classmethod
- def _push(cls, snapbuild, upload_id):
+ def _push(cls, snapbuild, upload_id, components={}):
"""Create a new store upload based on the uploaded file."""
snap = snapbuild.snap
assert snap.can_upload_to_store
@@ -305,6 +305,9 @@ class SnapStoreClient:
"series": snap.store_series.name,
"built_at": snapbuild.date_started.isoformat(),
}
+
+ if components:
+ data["components"] = components
# The security proxy is useless and breaks JSON serialisation.
channels = removeSecurityProxy(snap.store_channels)
if channels:
@@ -343,10 +346,10 @@ class SnapStoreClient:
raise cls._makeSnapStoreError(UploadFailedResponse, e)
@classmethod
- def push(cls, snapbuild, upload_id):
+ def push(cls, snapbuild, upload_id, components={}):
"""See `ISnapStoreClient`."""
return cls.refreshIfNecessary(
- snapbuild.snap, cls._push, snapbuild, upload_id
+ snapbuild.snap, cls._push, snapbuild, upload_id, components
)
@classmethod
diff --git a/lib/lp/snappy/tests/test_snapbuildjob.py b/lib/lp/snappy/tests/test_snapbuildjob.py
index 7e69f3c..c52741d 100644
--- a/lib/lp/snappy/tests/test_snapbuildjob.py
+++ b/lib/lp/snappy/tests/test_snapbuildjob.py
@@ -61,10 +61,32 @@ def run_isolated_jobs(jobs):
transaction.abort()
+class FakeMethodWithCounter(FakeMethod):
+ """Method to throw error after a given number of calls"""
+
+ counter = -1
+
+ def __init__(self, counter=-1):
+ self.counter = counter
+ super().__init__(result=1, failure=None)
+
+ def __call__(self, *args, **kwargs):
+ if self.call_count == self.counter:
+ self.failure = UploadFailedResponse("Proxy error", can_retry=True)
+ self.result = None
+ else:
+ self.result = 1
+ self.failure = None
+ return super().__call__(*args, **kwargs)
+
+
@implementer(ISnapStoreClient)
class FakeSnapStoreClient:
- def __init__(self):
- self.uploadFile = FakeMethod()
+ def __init__(self, counter=-1):
+ if counter >= 0:
+ self.uploadFile = FakeMethodWithCounter(counter)
+ else:
+ self.uploadFile = FakeMethod()
self.push = FakeMethod()
self.checkStatus = FakeMethod()
@@ -144,6 +166,32 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
)
return snapbuild
+ def makeSnapBuildWithComponents(self, num_components):
+ # Make a build with a builder, a file, and a webhook.
+ snapbuild = self.factory.makeSnapBuild(
+ builder=self.factory.makeBuilder()
+ )
+ snapbuild.updateStatus(BuildStatus.FULLYBUILT)
+ snap_lfa = self.factory.makeLibraryFileAlias(
+ filename="test-snap_0_all.snap", content=b"dummy snap content"
+ )
+
+ for i in range(num_components):
+ self.factory.makeSnapFile(
+ snapbuild=snapbuild, libraryfile=snap_lfa
+ )
+ component_lfa = self.factory.makeLibraryFileAlias(
+ filename="test-snap+somecomponent" + str(i) + "_0.comp",
+ content=b"component",
+ )
+ self.factory.makeSnapFile(
+ snapbuild=snapbuild, libraryfile=component_lfa
+ )
+ self.factory.makeWebhook(
+ target=snapbuild.snap, event_types=["snap:build:0.1"]
+ )
+ return snapbuild
+
def assertWebhookDeliveries(
self, snapbuild, expected_store_upload_statuses, logger
):
@@ -207,7 +255,101 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertThat(
client.uploadFile.calls, FileUploaded("test-snap.snap")
)
- self.assertEqual([((snapbuild, 1), {})], client.push.calls)
+ self.assertEqual([((snapbuild, 1, {}), {})], client.push.calls)
+ self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+ self.assertContentEqual([job], snapbuild.store_upload_jobs)
+ self.assertEqual(self.store_url, job.store_url)
+ self.assertEqual(1, job.store_revision)
+ self.assertEqual(1, snapbuild.store_upload_revision)
+ self.assertIsNone(job.error_message)
+ self.assertEqual([], pop_notifications())
+ self.assertWebhookDeliveries(
+ snapbuild, ["Pending", "Uploaded"], logger
+ )
+
+ def test_run_with_component(self):
+ # The job uploads the build to the storage with its component
+ # and then push it to the store and records the store URL
+ # and revision.
+ logger = self.useFixture(FakeLogger())
+ snapbuild = self.makeSnapBuildWithComponents(1)
+ self.assertContentEqual([], snapbuild.store_upload_jobs)
+ job = SnapStoreUploadJob.create(snapbuild)
+ client = FakeSnapStoreClient()
+ client.uploadFile.result = 1
+ client.push.result = self.status_url
+ client.checkStatus.result = (self.store_url, 1)
+ self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+ run_isolated_jobs([job])
+
+ # Check if uploadFile is called for snap and its component.
+ self.assertThat(
+ [client.uploadFile.calls[0]], FileUploaded("test-snap_0_all.snap")
+ )
+ self.assertThat(
+ [client.uploadFile.calls[1]],
+ FileUploaded("test-snap+somecomponent0_0.comp"),
+ )
+ self.assertEqual(
+ [((snapbuild, 1, {"somecomponent0": 1}), {})],
+ client.push.calls,
+ )
+ self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+ self.assertContentEqual([job], snapbuild.store_upload_jobs)
+ self.assertEqual(self.store_url, job.store_url)
+ self.assertEqual(1, job.store_revision)
+ self.assertEqual(1, snapbuild.store_upload_revision)
+ self.assertIsNone(job.error_message)
+ self.assertEqual([], pop_notifications())
+ self.assertWebhookDeliveries(
+ snapbuild, ["Pending", "Uploaded"], logger
+ )
+
+ def test_run_with_multiple_components(self):
+ # The job uploads the build to the storage with its components
+ # and then push it to the store and records the store URL
+ # and revision.
+ logger = self.useFixture(FakeLogger())
+ snapbuild = self.makeSnapBuildWithComponents(2)
+ self.assertContentEqual([], snapbuild.store_upload_jobs)
+ job = SnapStoreUploadJob.create(snapbuild)
+ client = FakeSnapStoreClient()
+ client.uploadFile.result = 1
+ client.push.result = self.status_url
+ client.checkStatus.result = (self.store_url, 1)
+ self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+ run_isolated_jobs([job])
+
+ # Check if uploadFile is called for snap and its components.
+ self.assertThat(
+ [client.uploadFile.calls[0]], FileUploaded("test-snap_0_all.snap")
+ )
+ self.assertThat(
+ [client.uploadFile.calls[1]],
+ FileUploaded("test-snap+somecomponent0_0.comp"),
+ )
+ self.assertThat(
+ [client.uploadFile.calls[2]],
+ FileUploaded("test-snap+somecomponent1_0.comp"),
+ )
+ self.assertEqual(
+ [
+ (
+ (
+ snapbuild,
+ 1,
+ {
+ "somecomponent0": 1,
+ "somecomponent1": 1,
+ },
+ ),
+ {},
+ )
+ ],
+ client.push.calls,
+ )
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
@@ -268,7 +410,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertThat(
client.uploadFile.calls, FileUploaded("test-snap.snap")
)
- self.assertEqual([((snapbuild, 1), {})], client.push.calls)
+ self.assertEqual([((snapbuild, 1, {}), {})], client.push.calls)
self.assertEqual([], client.checkStatus.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
@@ -313,6 +455,106 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
snapbuild, ["Pending", "Failed to upload"], logger
)
+ def test_run_502_retries_with_components(self):
+ # A run that gets a 502 error from the store schedules itself to be
+ # retried.
+ logger = self.useFixture(FakeLogger())
+ snapbuild = self.makeSnapBuildWithComponents(2)
+ self.assertContentEqual([], snapbuild.store_upload_jobs)
+ job = SnapStoreUploadJob.create(snapbuild)
+
+ # The error raises on the third uploadFile call.
+ client = FakeSnapStoreClient(2)
+ client.uploadFile.result = 1
+ self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+ run_isolated_jobs([job])
+ self.assertThat(
+ [client.uploadFile.calls[0]], FileUploaded("test-snap_0_all.snap")
+ )
+ self.assertThat(
+ [client.uploadFile.calls[1]],
+ FileUploaded("test-snap+somecomponent0_0.comp"),
+ )
+ self.assertThat(
+ [client.uploadFile.calls[2]],
+ FileUploaded("test-snap+somecomponent1_0.comp"),
+ )
+ self.assertEqual([], client.push.calls)
+ self.assertEqual([], client.checkStatus.calls)
+ self.assertContentEqual([job], snapbuild.store_upload_jobs)
+
+ # Component upload is incomplete.
+ self.assertEqual(
+ {
+ "somecomponent0": 1,
+ "somecomponent1": None,
+ },
+ job.components_ids,
+ )
+ self.assertIsNone(job.store_url)
+ self.assertIsNone(job.store_revision)
+ self.assertIsNone(snapbuild.store_upload_revision)
+ self.assertIsNone(job.error_message)
+ self.assertEqual([], pop_notifications())
+ self.assertEqual(JobStatus.WAITING, job.job.status)
+ self.assertWebhookDeliveries(snapbuild, ["Pending"], logger)
+
+ # Try again. The upload part of the job is retried, and this time
+ # it succeeds.
+ job.scheduled_start = None
+ client.uploadFile.counter = -1
+ client.uploadFile.failure = None
+ client.uploadFile.result = 1
+ client.push.result = self.status_url
+ client.checkStatus.result = (self.store_url, 1)
+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+ run_isolated_jobs([job])
+
+ # The last call should contains just the component that failed.
+ self.assertEqual(len(client.uploadFile.calls), 4)
+ self.assertThat(
+ [client.uploadFile.calls[3]],
+ FileUploaded("test-snap+somecomponent1_0.comp"),
+ )
+
+ self.assertEqual(
+ {
+ "somecomponent0.comp": 1,
+ "somecomponent1.comp": 1,
+ },
+ job.components_ids,
+ )
+
+ # After that the snap is uploaded to the store.
+ self.assertEqual(
+ [
+ (
+ (
+ snapbuild,
+ 1,
+ {
+ "somecomponent0": 1,
+ "somecomponent1": 1,
+ },
+ ),
+ {},
+ )
+ ],
+ client.push.calls,
+ )
+ self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+ self.assertContentEqual([job], snapbuild.store_upload_jobs)
+ self.assertEqual(self.store_url, job.store_url)
+ self.assertEqual(1, job.store_revision)
+ self.assertEqual(1, snapbuild.store_upload_revision)
+ self.assertIsNone(job.error_message)
+ self.assertEqual([], pop_notifications())
+ self.assertEqual(JobStatus.COMPLETED, job.job.status)
+ self.assertWebhookDeliveries(
+ snapbuild, ["Pending", "Uploaded"], logger
+ )
+
def test_run_502_retries(self):
# A run that gets a 502 error from the store schedules itself to be
# retried.
@@ -353,7 +595,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertThat(
client.uploadFile.calls, FileUploaded("test-snap.snap")
)
- self.assertEqual([((snapbuild, 1), {})], client.push.calls)
+ self.assertEqual([((snapbuild, 1, {}), {})], client.push.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
@@ -388,7 +630,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertThat(
client.uploadFile.calls, FileUploaded("test-snap.snap")
)
- self.assertEqual([((snapbuild, 1), {})], client.push.calls)
+ self.assertEqual([((snapbuild, 1, {}), {})], client.push.calls)
self.assertEqual([], client.checkStatus.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
@@ -524,7 +766,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertThat(
client.uploadFile.calls, FileUploaded("test-snap.snap")
)
- self.assertEqual([((snapbuild, 2), {})], client.push.calls)
+ self.assertEqual([((snapbuild, 2, {}), {})], client.push.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
@@ -586,7 +828,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertThat(
client.uploadFile.calls, FileUploaded("test-snap.snap")
)
- self.assertEqual([((snapbuild, 2), {})], client.push.calls)
+ self.assertEqual([((snapbuild, 2, {}), {})], client.push.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
@@ -655,7 +897,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertThat(
client.uploadFile.calls, FileUploaded("test-snap.snap")
)
- self.assertEqual([((snapbuild, 1), {})], client.push.calls)
+ self.assertEqual([((snapbuild, 1, {}), {})], client.push.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
@@ -684,7 +926,7 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
self.assertThat(
client.uploadFile.calls, FileUploaded("test-snap.snap")
)
- self.assertEqual([((snapbuild, 1), {})], client.push.calls)
+ self.assertEqual([((snapbuild, 1, {}), {})], client.push.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
Follow ups
