launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-store-upload-selective into lp:launchpad.

Commit message:
Only upload *.snap files to the store, not e.g. manifests.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1625117 in Launchpad itself: "store uploads fail since os snaps publish manifest files"
  https://bugs.launchpad.net/launchpad/+bug/1625117

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-store-upload-selective/+merge/306100

Only upload *.snap files to the store, not e.g. manifests.

archiveuploader already checks that there is at least one *.snap file.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-store-upload-selective into lp:launchpad.

=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
--- lib/lp/snappy/model/snapstoreclient.py	2016-09-16 10:06:13 +0000
+++ lib/lp/snappy/model/snapstoreclient.py	2016-09-19 13:45:51 +0000
@@ -171,7 +171,7 @@
             encoder = MultipartEncoder(
                 fields={
                     "binary": (
-                        "filename", lfa_wrapper, "application/octet-stream"),
+                        lfa.filename, lfa_wrapper, "application/octet-stream"),
                     })
             # XXX cjwatson 2016-05-09: This should add timeline information,
             # but that's currently difficult in jobs.
@@ -222,6 +222,8 @@
         """See `ISnapStoreClient`."""
         assert snapbuild.snap.can_upload_to_store
         for _, lfa, lfc in snapbuild.getFiles():
+            if not lfa.filename.endswith(".snap"):
+                continue
             upload_data = self._uploadFile(lfa, lfc)
             try:
                 return self._uploadApp(snapbuild.snap, upload_data)

=== modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
--- lib/lp/snappy/tests/test_snapstoreclient.py	2016-09-16 10:06:13 +0000
+++ lib/lp/snappy/tests/test_snapstoreclient.py	2016-09-19 13:45:51 +0000
@@ -31,6 +31,7 @@
     KeysEqual,
     Matcher,
     MatchesDict,
+    MatchesListwise,
     MatchesStructure,
     Mismatch,
     StartsWith,
@@ -180,6 +181,7 @@
         self.pushConfig(
             "launchpad", openid_provider_root="http://sso.example/";)
         self.client = getUtility(ISnapStoreClient)
+        self.unscanned_upload_requests = []
 
     def _make_store_secrets(self):
         self.root_key = hashlib.sha256(
@@ -200,7 +202,7 @@
 
     @urlmatch(path=r".*/unscanned-upload/$")
     def _unscanned_upload_handler(self, url, request):
-        self.unscanned_upload_request = request
+        self.unscanned_upload_requests.append(request)
         return {
             "status_code": 200,
             "content": {"successful": True, "upload_id": 1},
@@ -301,8 +303,13 @@
             store_series=self.factory.makeSnappySeries(name="rolling"),
             store_name="test-snap", store_secrets=self._make_store_secrets())
         snapbuild = self.factory.makeSnapBuild(snap=snap)
-        lfa = self.factory.makeLibraryFileAlias(content="dummy snap content")
-        self.factory.makeSnapFile(snapbuild=snapbuild, libraryfile=lfa)
+        snap_lfa = self.factory.makeLibraryFileAlias(
+            filename="test-snap.snap", content="dummy snap content")
+        self.factory.makeSnapFile(snapbuild=snapbuild, libraryfile=snap_lfa)
+        manifest_lfa = self.factory.makeLibraryFileAlias(
+            filename="test-snap.manifest", content="dummy manifest content")
+        self.factory.makeSnapFile(
+            snapbuild=snapbuild, libraryfile=manifest_lfa)
         transaction.commit()
         with dbuser(config.ISnapStoreUploadJobSource.dbuser):
             with HTTMock(self._unscanned_upload_handler,
@@ -310,15 +317,16 @@
                 self.assertEqual(
                     "http://sca.example/dev/api/snaps/1/builds/1/status";,
                     self.client.upload(snapbuild))
-        self.assertThat(self.unscanned_upload_request, RequestMatches(
-            url=Equals("http://updown.example/unscanned-upload/";),
-            method=Equals("POST"),
-            form_data={
-                "binary": MatchesStructure.byEquality(
-                    name="binary", filename="filename",
-                    value="dummy snap content",
-                    type="application/octet-stream",
-                    )}))
+        self.assertThat(self.unscanned_upload_requests, MatchesListwise([
+            RequestMatches(
+                url=Equals("http://updown.example/unscanned-upload/";),
+                method=Equals("POST"),
+                form_data={
+                    "binary": MatchesStructure.byEquality(
+                        name="binary", filename="test-snap.snap",
+                        value="dummy snap content",
+                        type="application/octet-stream",
+                        )})]))
         self.assertThat(self.snap_push_request, RequestMatches(
             url=Equals("http://sca.example/dev/api/snap-push/";),
             method=Equals("POST"),
@@ -343,7 +351,8 @@
             store_series=self.factory.makeSnappySeries(name="rolling"),
             store_name="test-snap", store_secrets=store_secrets)
         snapbuild = self.factory.makeSnapBuild(snap=snap)
-        lfa = self.factory.makeLibraryFileAlias(content="dummy snap content")
+        lfa = self.factory.makeLibraryFileAlias(
+            filename="test-snap.snap", content="dummy snap content")
         self.factory.makeSnapFile(snapbuild=snapbuild, libraryfile=lfa)
         transaction.commit()
         with dbuser(config.ISnapStoreUploadJobSource.dbuser):
@@ -372,7 +381,8 @@
             store_series=self.factory.makeSnappySeries(name="rolling"),
             store_name="test-snap", store_secrets=store_secrets)
         snapbuild = self.factory.makeSnapBuild(snap=snap)
-        lfa = self.factory.makeLibraryFileAlias(content="dummy snap content")
+        lfa = self.factory.makeLibraryFileAlias(
+            filename="test-snap.snap", content="dummy snap content")
         self.factory.makeSnapFile(snapbuild=snapbuild, libraryfile=lfa)
         transaction.commit()
         with dbuser(config.ISnapStoreUploadJobSource.dbuser):