launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snapcraft-yaml-not-found-error into lp:launchpad.

Commit message:
Remove spurious u'...' from 'Cannot find snapcraft.yaml in ...' error messages.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snapcraft-yaml-not-found-error/+merge/356307
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snapcraft-yaml-not-found-error into lp:launchpad.

=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py	2018-09-27 13:51:42 +0000
+++ lib/lp/snappy/browser/snap.py	2018-10-09 10:03:37 +0000
@@ -43,7 +43,6 @@
 from lp.app.browser.lazrjs import InlinePersonEditPickerWidget
 from lp.app.browser.tales import format_link
 from lp.app.enums import PRIVATE_INFORMATION_TYPES
-from lp.app.errors import NotFoundError
 from lp.app.interfaces.informationtype import IInformationType
 from lp.app.widgets.itemswidgets import (
     LabeledMultiCheckBoxWidget,
@@ -88,6 +87,7 @@
     CannotParseSnapcraftYaml,
     ISnap,
     ISnapSet,
+    MissingSnapcraftYaml,
     NoSuchSnap,
     SNAP_PRIVATE_FEATURE_FLAG,
     SnapBuildAlreadyPending,
@@ -476,7 +476,7 @@
             try:
                 snapcraft_data = getUtility(ISnapSet).getSnapcraftYaml(
                     self.context, logger=log)
-            except (NotFoundError, CannotFetchSnapcraftYaml,
+            except (MissingSnapcraftYaml, CannotFetchSnapcraftYaml,
                     CannotParseSnapcraftYaml):
                 pass
             else:

=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py	2018-09-27 13:51:42 +0000
+++ lib/lp/snappy/interfaces/snap.py	2018-10-09 10:03:37 +0000
@@ -19,6 +19,7 @@
     'ISnapEdit',
     'ISnapSet',
     'ISnapView',
+    'MissingSnapcraftYaml',
     'NoSourceForSnap',
     'NoSuchSnap',
     'SNAP_PRIVATE_FEATURE_FLAG',
@@ -247,6 +248,14 @@
             "because %s is not set." % field)
 
 
+class MissingSnapcraftYaml(Exception):
+    """The repository for this snap package does not have a snapcraft.yaml."""
+
+    def __init__(self, branch_name):
+        super(MissingSnapcraftYaml, self).__init__(
+            "Cannot find snapcraft.yaml in %s" % branch_name)
+
+
 class CannotFetchSnapcraftYaml(Exception):
     """Launchpad cannot fetch this snap package's snapcraft.yaml."""
 
@@ -981,7 +990,7 @@
         :param logger: An optional logger.
 
         :return: The package's parsed snapcraft.yaml.
-        :raises NotFoundError: if this package has no snapcraft.yaml.
+        :raises MissingSnapcraftYaml: if this package has no snapcraft.yaml.
         :raises CannotFetchSnapcraftYaml: if it was not possible to fetch
             snapcraft.yaml from the code hosting backend for some other
             reason.

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2018-10-03 00:48:36 +0000
+++ lib/lp/snappy/model/snap.py	2018-10-09 10:03:37 +0000
@@ -144,6 +144,7 @@
     ISnap,
     ISnapBuildRequest,
     ISnapSet,
+    MissingSnapcraftYaml,
     NoSourceForSnap,
     NoSuchSnap,
     SNAP_PRIVATE_FEATURE_FLAG,
@@ -1171,10 +1172,11 @@
                 except (BranchFileNotFound, GitRepositoryBlobNotFound):
                     pass
             else:
-                msg = "Cannot find snapcraft.yaml in %s"
                 if logger is not None:
-                    logger.exception(msg, context.unique_name)
-                raise NotFoundError(msg % context.unique_name)
+                    logger.exception(
+                        "Cannot find snapcraft.yaml in %s",
+                        context.unique_name)
+                raise MissingSnapcraftYaml(context.unique_name)
         except GitRepositoryBlobUnsupportedRemote as e:
             raise CannotFetchSnapcraftYaml(str(e), unsupported_remote=True)
         except (BranchHostingFault, GitRepositoryScanFault) as e:

=== modified file 'lib/lp/snappy/model/snapjob.py'
--- lib/lp/snappy/model/snapjob.py	2018-09-13 15:21:05 +0000
+++ lib/lp/snappy/model/snapjob.py	2018-10-09 10:03:37 +0000
@@ -57,6 +57,7 @@
 from lp.snappy.interfaces.snap import (
     CannotFetchSnapcraftYaml,
     CannotParseSnapcraftYaml,
+    MissingSnapcraftYaml,
     )
 from lp.snappy.interfaces.snapjob import (
     ISnapJob,
@@ -175,7 +176,7 @@
 
     class_job_type = SnapJobType.REQUEST_BUILDS
 
-    user_error_types = (CannotParseSnapcraftYaml, NotFoundError)
+    user_error_types = (CannotParseSnapcraftYaml, MissingSnapcraftYaml)
     retry_error_types = (CannotFetchSnapcraftYaml,)
 
     max_retries = 5

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2018-09-27 13:51:42 +0000
+++ lib/lp/snappy/tests/test_snap.py	2018-10-09 10:03:37 +0000
@@ -1690,16 +1690,14 @@
             GitRepositoryBlobNotFound("dummy", "snap/snapcraft.yaml"))
         self.assertEqual(
             [], getUtility(ISnapSet).makeAutoBuilds(logger=logger))
-        # The u'...' here is a little odd, but that's how KeyError (and thus
-        # NotFoundError) stringifies.
         expected_log_entries = [
             "DEBUG Scheduling builds of snap package %s/%s" % (
                 snap1.owner.name, snap1.name),
-            "ERROR  - %s/%s: u'Cannot find snapcraft.yaml in %s'" % (
+            "ERROR  - %s/%s: Cannot find snapcraft.yaml in %s" % (
                 snap1.owner.name, snap1.name, snap1.git_ref.unique_name),
             "DEBUG Scheduling builds of snap package %s/%s" % (
                 snap2.owner.name, snap2.name),
-            "ERROR  - %s/%s: u'Cannot find snapcraft.yaml in %s'" % (
+            "ERROR  - %s/%s: Cannot find snapcraft.yaml in %s" % (
                 snap2.owner.name, snap2.name, snap2.git_ref.unique_name),
             ]
         self.assertThat(