launchpad-reviewers team mailing list archive

[Maximiliano Bertacchini <maximiliano.bertacchini@xxxxxxxxxxxxx>]

Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/lp-1729580 into lp:launchpad.

Commit message:
Expose extended error messages (with external link) for snap build jobs (LP: #1729580).

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/lp-1729580/+merge/337825

Expose extended error messages (with external link) for snap build jobs (LP: #1729580).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/lp-1729580 into lp:launchpad.

=== modified file 'lib/lp/snappy/interfaces/snapbuild.py'
--- lib/lp/snappy/interfaces/snapbuild.py	2017-04-27 16:22:37 +0000
+++ lib/lp/snappy/interfaces/snapbuild.py	2018-02-15 22:12:44 +0000
@@ -40,6 +40,7 @@
     Choice,
     Datetime,
     Int,
+    Text,
     TextLine,
     )
 
@@ -213,6 +214,15 @@
             "this snap build to the store."),
         required=False, readonly=True))
 
+    store_upload_error_messages = exported(Text(
+        title=_("Store upload error messages"),
+        description=_(
+            "A list of dict(message, link) where message is an error "
+            "description and link, if any, is an external link to extra "
+            "details, from the last attempt to upload this snap build "
+            "to the store."),
+        required=False, readonly=True))
+
     def getFiles():
         """Retrieve the build's `ISnapFile` records.
 

=== modified file 'lib/lp/snappy/interfaces/snapstoreclient.py'
--- lib/lp/snappy/interfaces/snapstoreclient.py	2017-06-14 10:25:23 +0000
+++ lib/lp/snappy/interfaces/snapstoreclient.py	2018-02-15 22:12:44 +0000
@@ -29,10 +29,12 @@
 
 class SnapStoreError(Exception):
 
-    def __init__(self, message="", detail=None, can_retry=False):
+    def __init__(
+            self, message="", detail=None, messages=None, can_retry=False):
         super(SnapStoreError, self).__init__(message)
         self.message = message
         self.detail = detail
+        self.messages = messages
         self.can_retry = can_retry
 
 

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2018-01-23 10:59:44 +0000
+++ lib/lp/snappy/model/snapbuild.py	2018-02-15 22:12:44 +0000
@@ -456,6 +456,11 @@
         job = self.last_store_upload_job
         return job and job.error_message
 
+    @property
+    def store_upload_error_messages(self):
+        job = self.last_store_upload_job
+        return job and job.error_messages
+
     def scheduleStoreUpload(self):
         """See `ISnapBuild`."""
         if not self.snap.can_upload_to_store:

=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py	2017-06-29 16:58:01 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2018-02-15 22:12:44 +0000
@@ -232,6 +232,16 @@
         self.metadata["error_detail"] = detail
 
     @property
+    def error_messages(self):
+        """See `ISnapStoreUploadJob`."""
+        return self.metadata.get("error_messages")
+
+    @error_messages.setter
+    def error_messages(self, messages):
+        """See `ISnapStoreUploadJob`."""
+        self.metadata["error_messages"] = messages
+
+    @property
     def store_url(self):
         """See `ISnapStoreUploadJob`."""
         return self.metadata.get("store_url")
@@ -326,6 +336,7 @@
                     self.attempt_count <= self.max_retries):
                 raise RetryableSnapStoreError(e.message, detail=e.detail)
             self.error_message = str(e)
+            self.error_messages = getattr(e, "messages", None)
             self.error_detail = getattr(e, "detail", None)
             if isinstance(e, UnauthorizedUploadResponse):
                 mailer = SnapBuildMailer.forUnauthorizedUpload(self.snapbuild)

=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
--- lib/lp/snappy/model/snapstoreclient.py	2017-06-14 10:25:23 +0000
+++ lib/lp/snappy/model/snapstoreclient.py	2018-02-15 22:12:44 +0000
@@ -316,7 +316,11 @@
             elif "errors" in response_data:
                 error_message = "\n".join(
                     error["message"] for error in response_data["errors"])
-                raise ScanFailedResponse(error_message)
+                error_messages = [
+                    {"message": error["message"], "link": error.get("link")}
+                    for error in response_data["errors"]]
+                raise ScanFailedResponse(
+                    error_message, messages=error_messages)
             elif not response_data["can_release"]:
                 return response_data["url"], None
             else:

=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py	2017-06-29 18:06:03 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2018-02-15 22:12:44 +0000
@@ -31,7 +31,7 @@
     )
 from lp.snappy.interfaces.snapstoreclient import (
     BadRefreshResponse,
-    BadScanStatusResponse,
+    ScanFailedResponse,
     ISnapStoreClient,
     ReleaseFailedResponse,
     UnauthorizedUploadResponse,
@@ -433,7 +433,11 @@
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
         client.upload.result = self.status_url
-        client.checkStatus.failure = BadScanStatusResponse("Scan failed.")
+        client.checkStatus.failure = ScanFailedResponse(
+            "Scan failed.\nConfinement not allowed.",
+            messages=[
+                {"message": "Scan failed.", "link": "link1"},
+                {"message": "Confinement not allowed.", "link": "link2"}])
         self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
         with dbuser(config.ISnapStoreUploadJobSource.dbuser):
             JobRunner([job]).runAll()
@@ -443,7 +447,12 @@
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertIsNone(job.store_url)
         self.assertIsNone(job.store_revision)
-        self.assertEqual("Scan failed.", job.error_message)
+        self.assertEqual(
+            "Scan failed.\nConfinement not allowed.", job.error_message)
+        self.assertEqual([
+            {"message": "Scan failed.", "link": "link1"},
+            {"message": "Confinement not allowed.", "link": "link2"}],
+            job.error_messages)
         [notification] = pop_notifications()
         self.assertEqual(
             config.canonical.noreply_from_address, notification["From"])

=== modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
--- lib/lp/snappy/tests/test_snapstoreclient.py	2017-06-14 10:25:23 +0000
+++ lib/lp/snappy/tests/test_snapstoreclient.py	2018-02-15 22:12:44 +0000
@@ -581,6 +581,7 @@
                     "errors": [
                         {"code": None,
                          "message": "You cannot use that reserved namespace.",
+                         "link": "http://example.com";
                          }],
                     }}