launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #20699
[Merge] lp:~cjwatson/launchpad/snap-channels-job into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-channels-job into lp:launchpad with lp:~cjwatson/launchpad/snap-channels-store-client as a prerequisite.
Commit message:
Automatically release snap packages that have store_channels set.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1597819 in Launchpad itself: "Add option to automatically release snap packages to channels after upload"
https://bugs.launchpad.net/launchpad/+bug/1597819
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-channels-job/+merge/298810
Automatically release snap packages that have store_channels set.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-channels-job into lp:launchpad.
=== added file 'lib/lp/snappy/emailtemplates/snapbuild-manualreview.txt'
--- lib/lp/snappy/emailtemplates/snapbuild-manualreview.txt 1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/emailtemplates/snapbuild-manualreview.txt 2016-06-30 17:00:36 +0000
@@ -0,0 +1,5 @@
+This snap package could not be released automatically because it was held
+for manual review. Once it has been approved, you will need to release it
+manually from here:
+
+ %(store_url)s
=== added file 'lib/lp/snappy/emailtemplates/snapbuild-releasefailed.txt'
--- lib/lp/snappy/emailtemplates/snapbuild-releasefailed.txt 1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/emailtemplates/snapbuild-releasefailed.txt 2016-06-30 17:00:36 +0000
@@ -0,0 +1,7 @@
+Launchpad asked the store to release this snap package, but it failed:
+
+ %(store_error_message)s
+
+You can try to release it manually here:
+
+ %(store_url)s
=== modified file 'lib/lp/snappy/mail/snapbuild.py'
--- lib/lp/snappy/mail/snapbuild.py 2016-06-21 14:51:06 +0000
+++ lib/lp/snappy/mail/snapbuild.py 2016-06-30 17:00:36 +0000
@@ -60,6 +60,34 @@
config.canonical.noreply_from_address,
"snap-build-upload-scan-failed", build)
+ @classmethod
+ def forManualReview(cls, build):
+ """Create a mailer for notifying about manual review.
+
+ :param build: The relevant build.
+ """
+ requester = build.requester
+ recipients = {requester: RecipientReason.forBuildRequester(requester)}
+ return cls(
+ "%(snap_name)s held for manual review",
+ "snapbuild-manualreview.txt", recipients,
+ config.canonical.noreply_from_address,
+ "snap-build-release-manual-review", build)
+
+ @classmethod
+ def forReleaseFailure(cls, build):
+ """Create a mailer for notifying about store release failures.
+
+ :param build: The relevant build.
+ """
+ requester = build.requester
+ recipients = {requester: RecipientReason.forBuildRequester(requester)}
+ return cls(
+ "Store release failed for %(snap_name)s",
+ "snapbuild-releasefailed.txt", recipients,
+ config.canonical.noreply_from_address,
+ "snap-build-release-failed", build)
+
def __init__(self, subject, template_name, recipients, from_address,
notification_type, build):
super(SnapBuildMailer, self).__init__(
@@ -77,10 +105,12 @@
"""See `BaseMailer`."""
build = self.build
upload_job = build.store_upload_jobs.first()
- if upload_job is None or upload_job.error_message is None:
+ if upload_job is None:
error_message = ""
+ store_url = ""
else:
- error_message = upload_job.error_message
+ error_message = upload_job.error_message or ""
+ store_url = upload_job.store_url or ""
params = super(SnapBuildMailer, self)._getTemplateParams(
email, recipient)
params.update({
@@ -100,6 +130,7 @@
"snap_authorize_url": canonical_url(
build.snap, view_name="+authorize"),
"store_error_message": error_message,
+ "store_url": store_url,
})
if build.duration is not None:
duration_formatter = DurationFormatterAPI(build.duration)
=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py 2016-06-21 14:51:06 +0000
+++ lib/lp/snappy/model/snapbuildjob.py 2016-06-30 17:00:36 +0000
@@ -50,8 +50,10 @@
ISnapStoreUploadJobSource,
)
from lp.snappy.interfaces.snapstoreclient import (
+ BadReleaseResponse,
BadScanStatusResponse,
ISnapStoreClient,
+ ReleaseFailedResponse,
ScanFailedResponse,
UnauthorizedUploadResponse,
UploadNotScannedYetResponse,
@@ -157,6 +159,10 @@
return oops_vars
+class ManualReview(Exception):
+ pass
+
+
@implementer(ISnapStoreUploadJob)
@provider(ISnapStoreUploadJobSource)
class SnapStoreUploadJob(SnapBuildJobDerived):
@@ -164,7 +170,12 @@
class_job_type = SnapBuildJobType.STORE_UPLOAD
- user_error_types = (UnauthorizedUploadResponse, ScanFailedResponse)
+ user_error_types = (
+ UnauthorizedUploadResponse,
+ ScanFailedResponse,
+ ManualReview,
+ ReleaseFailedResponse,
+ )
# XXX cjwatson 2016-05-04: identify transient upload failures and retry
retry_error_types = (UploadNotScannedYetResponse,)
@@ -207,14 +218,19 @@
try:
if "status_url" not in self.metadata:
self.metadata["status_url"] = client.upload(self.snapbuild)
- self.store_url = client.checkStatus(self.metadata["status_url"])
+ if self.store_url is None:
+ self.store_url, self.metadata["store_revision"] = (
+ client.checkStatus(self.metadata["status_url"]))
+ if self.snapbuild.snap.store_channels:
+ if self.metadata["store_revision"] is None:
+ raise ManualReview(
+ "Package held for manual review on the store; "
+ "cannot release it automatically.")
+ client.release(self.snapbuild, self.metadata["store_revision"])
self.error_message = None
except self.retry_error_types:
raise
except Exception as e:
- # Abort work done so far, but make sure that we commit the error
- # message.
- transaction.abort()
self.error_message = str(e)
if isinstance(e, UnauthorizedUploadResponse):
mailer = SnapBuildMailer.forUnauthorizedUpload(self.snapbuild)
@@ -222,5 +238,14 @@
elif isinstance(e, (BadScanStatusResponse, ScanFailedResponse)):
mailer = SnapBuildMailer.forUploadScanFailure(self.snapbuild)
mailer.sendAll()
+ elif isinstance(e, ManualReview):
+ mailer = SnapBuildMailer.forManualReview(self.snapbuild)
+ mailer.sendAll()
+ elif isinstance(e, (BadReleaseResponse, ReleaseFailedResponse)):
+ mailer = SnapBuildMailer.forReleaseFailure(self.snapbuild)
+ mailer.sendAll()
+ # The normal job infrastructure will abort the transaction, but
+ # we want to commit instead: the only database changes we make
+ # are to this job's metadata and should be preserved.
transaction.commit()
raise
=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py 2016-06-21 14:51:06 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py 2016-06-30 17:00:36 +0000
@@ -20,6 +20,7 @@
ISnapStoreUploadJob,
)
from lp.snappy.interfaces.snapstoreclient import (
+ BadReleaseResponse,
BadScanStatusResponse,
ISnapStoreClient,
UnauthorizedUploadResponse,
@@ -47,6 +48,7 @@
def __init__(self):
self.upload = FakeMethod()
self.checkStatus = FakeMethod()
+ self.release = FakeMethod()
class TestSnapBuildJob(TestCaseWithFactory):
@@ -95,12 +97,13 @@
job = SnapStoreUploadJob.create(snapbuild)
client = FakeSnapStoreClient()
client.upload.result = self.status_url
- client.checkStatus.result = self.store_url
+ client.checkStatus.result = (self.store_url, 1)
self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
with dbuser(config.ISnapStoreUploadJobSource.dbuser):
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+ self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertIsNone(job.error_message)
@@ -118,6 +121,7 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([], client.checkStatus.calls)
+ self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertEqual("An upload failure", job.error_message)
@@ -138,6 +142,7 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([], client.checkStatus.calls)
+ self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertEqual("Authorization failed.", job.error_message)
@@ -178,6 +183,7 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+ self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.error_message)
@@ -190,11 +196,12 @@
client.upload.calls = []
client.checkStatus.calls = []
client.checkStatus.failure = None
- client.checkStatus.result = self.store_url
+ client.checkStatus.result = (self.store_url, 1)
with dbuser(config.ISnapStoreUploadJobSource.dbuser):
JobRunner([job]).runAll()
self.assertEqual([], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+ self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertIsNone(job.error_message)
@@ -216,6 +223,7 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+ self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertEqual("Scan failed.", job.error_message)
@@ -239,3 +247,114 @@
self.assertEqual(
"http://launchpad.dev/~requester/+snap/test-snap/+build/%d\n";
"You are the requester of the build.\n" % snapbuild.id, footer)
+
+ def test_run_release(self):
+ # A run configured to automatically release the package to certain
+ # channels does so.
+ snapbuild = self.factory.makeSnapBuild(
+ store_channels=["stable", "edge"])
+ self.assertContentEqual([], snapbuild.store_upload_jobs)
+ job = SnapStoreUploadJob.create(snapbuild)
+ client = FakeSnapStoreClient()
+ client.upload.result = self.status_url
+ client.checkStatus.result = (self.store_url, 1)
+ self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+ JobRunner([job]).runAll()
+ self.assertEqual([((snapbuild,), {})], client.upload.calls)
+ self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+ self.assertEqual([((snapbuild, 1), {})], client.release.calls)
+ self.assertContentEqual([job], snapbuild.store_upload_jobs)
+ self.assertEqual(self.store_url, job.store_url)
+ self.assertIsNone(job.error_message)
+ self.assertEqual([], pop_notifications())
+
+ def test_run_release_manual_review_notifies(self):
+ # A run configured to automatically release the package to certain
+ # channels but that encounters the manual review state on upload
+ # sends mail.
+ requester = self.factory.makePerson(name="requester")
+ snapbuild = self.factory.makeSnapBuild(
+ requester=requester, name="test-snap", owner=requester,
+ store_channels=["stable", "edge"])
+ self.assertContentEqual([], snapbuild.store_upload_jobs)
+ job = SnapStoreUploadJob.create(snapbuild)
+ client = FakeSnapStoreClient()
+ client.upload.result = self.status_url
+ client.checkStatus.result = (self.store_url, None)
+ self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+ JobRunner([job]).runAll()
+ self.assertEqual([((snapbuild,), {})], client.upload.calls)
+ self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+ self.assertEqual([], client.release.calls)
+ self.assertContentEqual([job], snapbuild.store_upload_jobs)
+ self.assertEqual(self.store_url, job.store_url)
+ self.assertEqual(
+ "Package held for manual review on the store; "
+ "cannot release it automatically.",
+ job.error_message)
+ [notification] = pop_notifications()
+ self.assertEqual(
+ config.canonical.noreply_from_address, notification["From"])
+ self.assertEqual(
+ "Requester <%s>" % requester.preferredemail.email,
+ notification["To"])
+ subject = notification["Subject"].replace("\n ", " ")
+ self.assertEqual("test-snap held for manual review", subject)
+ self.assertEqual(
+ "Requester", notification["X-Launchpad-Message-Rationale"])
+ self.assertEqual(
+ requester.name, notification["X-Launchpad-Message-For"])
+ self.assertEqual(
+ "snap-build-release-manual-review",
+ notification["X-Launchpad-Notification-Type"])
+ body, footer = notification.get_payload(decode=True).split("\n-- \n")
+ self.assertIn(self.store_url, body)
+ self.assertEqual(
+ "http://launchpad.dev/~requester/+snap/test-snap/+build/%d\n";
+ "You are the requester of the build.\n" % snapbuild.id, footer)
+
+ def test_run_release_failure_notifies(self):
+ # A run configured to automatically release the package to certain
+ # channels but that fails to do so sends mail.
+ requester = self.factory.makePerson(name="requester")
+ snapbuild = self.factory.makeSnapBuild(
+ requester=requester, name="test-snap", owner=requester,
+ store_channels=["stable", "edge"])
+ self.assertContentEqual([], snapbuild.store_upload_jobs)
+ job = SnapStoreUploadJob.create(snapbuild)
+ client = FakeSnapStoreClient()
+ client.upload.result = self.status_url
+ client.checkStatus.result = (self.store_url, 1)
+ client.release.failure = BadReleaseResponse("Failed to publish")
+ self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+ JobRunner([job]).runAll()
+ self.assertEqual([((snapbuild,), {})], client.upload.calls)
+ self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+ self.assertEqual([((snapbuild, 1), {})], client.release.calls)
+ self.assertContentEqual([job], snapbuild.store_upload_jobs)
+ self.assertEqual(self.store_url, job.store_url)
+ self.assertEqual("Failed to publish", job.error_message)
+ [notification] = pop_notifications()
+ self.assertEqual(
+ config.canonical.noreply_from_address, notification["From"])
+ self.assertEqual(
+ "Requester <%s>" % requester.preferredemail.email,
+ notification["To"])
+ subject = notification["Subject"].replace("\n ", " ")
+ self.assertEqual("Store release failed for test-snap", subject)
+ self.assertEqual(
+ "Requester", notification["X-Launchpad-Message-Rationale"])
+ self.assertEqual(
+ requester.name, notification["X-Launchpad-Message-For"])
+ self.assertEqual(
+ "snap-build-release-failed",
+ notification["X-Launchpad-Notification-Type"])
+ body, footer = notification.get_payload(decode=True).split("\n-- \n")
+ self.assertIn("Failed to publish", body)
+ self.assertIn(self.store_url, body)
+ self.assertEqual(
+ "http://launchpad.dev/~requester/+snap/test-snap/+build/%d\n";
+ "You are the requester of the build.\n" % snapbuild.id, footer)
Follow ups
