launchpad-reviewers team mailing list archive

[Colin Watson <mp+425407@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:ci-build-upload-job-mail into launchpad:master.

Commit message:
Notify requester by email when a CIBuildUploadJob fails

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/425407

Otherwise we have to dig through logs on users' behalf, which is no fun.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:ci-build-upload-job-mail into launchpad:master.

diff --git a/lib/lp/soyuz/model/archivejob.py b/lib/lp/soyuz/model/archivejob.py
index 3aca24b..58a4356 100644
--- a/lib/lp/soyuz/model/archivejob.py
+++ b/lib/lp/soyuz/model/archivejob.py
@@ -41,6 +41,7 @@ from lp.services.job.model.job import (
 from lp.services.job.runner import BaseRunnableJob
 from lp.services.librarian.interfaces.client import LibrarianServerError
 from lp.services.librarian.utils import copy_and_close
+from lp.services.mail.sendmail import format_address_for_person
 from lp.soyuz.enums import (
     ArchiveJobType,
     ArchiveRepositoryFormat,
@@ -202,6 +203,7 @@ class CIBuildUploadJob(ArchiveJobDerived):
 
     class_job_type = ArchiveJobType.CI_BUILD_UPLOAD
 
+    user_error_types = (ScanException,)
     retry_error_types = (LibrarianServerError,)
     max_retries = 3
 
@@ -274,6 +276,14 @@ class CIBuildUploadJob(ArchiveJobDerived):
                 )])
         return vars
 
+    def getOperationDescription(self):
+        """See `IRunnableJob`."""
+        return "uploading %s to %s" % (
+            self.ci_build.title, self.archive.reference)
+
+    def getErrorRecipients(self):
+        return [format_address_for_person(self.requester)]
+
     @property
     def ci_build(self):
         return getUtility(ICIBuildSet).getByID(self.metadata["ci_build_id"])
@@ -310,12 +320,13 @@ class CIBuildUploadJob(ArchiveJobDerived):
     def _scanFile(self, path):
         # XXX cjwatson 2022-06-10: We should probably start splitting this
         # up some more.
+        name = os.path.basename(path)
         if path.endswith(".whl"):
             try:
                 parsed_path = parse_wheel_filename(path)
                 wheel = Wheel(path)
             except Exception as e:
-                raise ScanException("Failed to scan %s" % path) from e
+                raise ScanException("Failed to scan %s" % name) from e
             return {
                 "name": wheel.name,
                 "version": wheel.version,
@@ -333,7 +344,7 @@ class CIBuildUploadJob(ArchiveJobDerived):
                     about = json.loads(
                         tar.extractfile("info/about.json").read().decode())
             except Exception as e:
-                raise ScanException("Failed to scan %s" % path) from e
+                raise ScanException("Failed to scan %s" % name) from e
             scanned = {"binpackageformat": BinaryPackageFormat.CONDA_V1}
             scanned.update(self._scanCondaMetadata(index, about))
             return scanned
@@ -352,7 +363,7 @@ class CIBuildUploadJob(ArchiveJobDerived):
                         about = json.loads(
                             tar.extractfile("info/about.json").read().decode())
             except Exception as e:
-                raise ScanException("Failed to scan %s" % path) from e
+                raise ScanException("Failed to scan %s" % name) from e
             scanned = {"binpackageformat": BinaryPackageFormat.CONDA_V2}
             scanned.update(self._scanCondaMetadata(index, about))
             return scanned
diff --git a/lib/lp/soyuz/tests/test_archivejob.py b/lib/lp/soyuz/tests/test_archivejob.py
index 922a267..5696a0c 100644
--- a/lib/lp/soyuz/tests/test_archivejob.py
+++ b/lib/lp/soyuz/tests/test_archivejob.py
@@ -6,6 +6,7 @@ import os.path
 from debian.deb822 import Changes
 from fixtures import MockPatch
 from testtools.matchers import (
+    ContainsDict,
     Equals,
     Is,
     MatchesSetwise,
@@ -16,6 +17,7 @@ import transaction
 from lp.code.enums import RevisionStatusArtifactType
 from lp.code.model.revisionstatus import RevisionStatusArtifact
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.config import config
 from lp.services.database.interfaces import IStore
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
@@ -661,6 +663,44 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
 
         self.assertEqual(JobStatus.COMPLETED, job.job.status)
 
+    def test_run_failed(self):
+        # A failed run sets the job status to FAILED and notifies the
+        # requester.
+        archive = self.factory.makeArchive(
+            repository_format=ArchiveRepositoryFormat.PYTHON)
+        distroseries = self.factory.makeDistroSeries(
+            distribution=archive.distribution)
+        das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        build = self.factory.makeCIBuild(distro_arch_series=das)
+        report = build.getOrCreateRevisionStatusReport("build:0")
+        path = "wheel-indep/dist/wheel_indep-0.0.1-py3-none-any.whl"
+        with open(datadir(path), mode="rb") as f:
+            # Use an invalid file name to force a scan failure.
+            report.attach(name="_invalid.whl", data=f.read())
+        job = CIBuildUploadJob.create(
+            build, build.git_repository.owner, archive, distroseries,
+            PackagePublishingPocket.RELEASE, target_channel="0.0.1/edge")
+        transaction.commit()
+
+        with dbuser(job.config.dbuser):
+            JobRunner([job]).runAll()
+
+        self.assertEqual(JobStatus.FAILED, job.job.status)
+        [notification] = self.assertEmailQueueLength(1)
+        self.assertThat(dict(notification), ContainsDict({
+            "From": Equals(config.canonical.noreply_from_address),
+            "To": Equals(
+                format_address_for_person(build.git_repository.owner)),
+            "Subject": Equals(
+                "Launchpad error while uploading %s to %s" %
+                (build.title, archive.reference)),
+            }))
+        self.assertEqual(
+            "Launchpad encountered an error during the following operation: "
+            "uploading %s to %s.  Failed to scan _invalid.whl" % (
+                build.title, archive.reference),
+            notification.get_payload(decode=True).decode())
+
 
 class TestViaCelery(TestCaseWithFactory):