launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-push-remove-old-status into lp:launchpad.

Commit message:
Remove handling of old snap-push response status URL.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-push-remove-old-status/+merge/305927

Remove handling of old snap-push response status URL.

SCA has returned status_details_url as well for some time, which is what we prefer, so we might as well drop the old cruft.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-push-remove-old-status into lp:launchpad.

=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
--- lib/lp/snappy/model/snapstoreclient.py	2016-07-12 15:01:54 +0000
+++ lib/lp/snappy/model/snapstoreclient.py	2016-09-16 10:30:38 +0000
@@ -208,10 +208,7 @@
                     snap.store_secrets["root"],
                     snap.store_secrets["discharge"]))
             response_data = response.json()
-            if "status_details_url" in response_data:
-                return response_data["status_details_url"]
-            else:
-                return response_data["status_url"]
+            return response_data["status_details_url"]
         except requests.HTTPError as e:
             if e.response.status_code == 401:
                 if (e.response.headers.get("WWW-Authenticate") ==
@@ -260,28 +257,16 @@
         try:
             response = urlfetch(status_url)
             response_data = response.json()
-            if "completed" in response_data:
-                # Old status format.
-                if not response_data["completed"]:
-                    raise UploadNotScannedYetResponse()
-                elif not response_data["application_url"]:
-                    raise ScanFailedResponse(response_data["message"])
-                else:
-                    return (
-                        response_data["application_url"],
-                        response_data["revision"])
+            if not response_data["processed"]:
+                raise UploadNotScannedYetResponse()
+            elif "errors" in response_data:
+                error_message = "\n".join(
+                    error["message"] for error in response_data["errors"])
+                raise ScanFailedResponse(error_message)
+            elif not response_data["can_release"]:
+                return response_data["url"], None
             else:
-                # New status format.
-                if not response_data["processed"]:
-                    raise UploadNotScannedYetResponse()
-                elif "errors" in response_data:
-                    error_message = "\n".join(
-                        error["message"] for error in response_data["errors"])
-                    raise ScanFailedResponse(error_message)
-                elif not response_data["can_release"]:
-                    return response_data["url"], None
-                else:
-                    return response_data["url"], response_data["revision"]
+                return response_data["url"], response_data["revision"]
         except requests.HTTPError as e:
             raise BadScanStatusResponse(e.args[0])
 

=== modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
--- lib/lp/snappy/tests/test_snapstoreclient.py	2016-07-15 17:11:09 +0000
+++ lib/lp/snappy/tests/test_snapstoreclient.py	2016-09-16 10:30:38 +0000
@@ -213,9 +213,6 @@
             "status_code": 202,
             "content": {
                 "success": True,
-                "status_url": (
-                    "http://sca.example/dev/api/";
-                    "click-scan-complete/updown/1/"),
                 "status_details_url": (
                     "http://sca.example/dev/api/snaps/1/builds/1/status";),
                 }}
@@ -388,32 +385,6 @@
         self.assertNotEqual(
             store_secrets["discharge"], snap.store_secrets["discharge"])
 
-    def test_upload_old_status_url(self):
-        @urlmatch(path=r".*/snap-push/$")
-        def snap_push_handler(url, request):
-            return {
-                "status_code": 202,
-                "content": {
-                    "success": True,
-                    "status_url": (
-                        "http://sca.example/dev/api/";
-                        "click-scan-complete/updown/1/"),
-                    }}
-
-        snap = self.factory.makeSnap(
-            store_upload=True,
-            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)
-        transaction.commit()
-        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
-            with HTTMock(self._unscanned_upload_handler, snap_push_handler):
-                self.assertEqual(
-                    "http://sca.example/dev/api/click-scan-complete/updown/1/";,
-                    self.client.upload(snapbuild))
-
     def test_refresh_discharge_macaroon(self):
         store_secrets = self._make_store_secrets()
         snap = self.factory.makeSnap(
@@ -432,59 +403,7 @@
         self.assertNotEqual(
             store_secrets["discharge"], snap.store_secrets["discharge"])
 
-    def test_checkStatus_old_pending(self):
-        @all_requests
-        def handler(url, request):
-            return {
-                "status_code": 200,
-                "content": {
-                    "completed": False, "application_url": "",
-                    "revision": None,
-                    "message": "Task 1 is waiting for execution.",
-                    "package_name": None,
-                    }}
-
-        status_url = "http://sca.example/dev/api/click-scan-complete/updown/1/";
-        with HTTMock(handler):
-            self.assertRaises(
-                UploadNotScannedYetResponse, self.client.checkStatus,
-                status_url)
-
-    def test_checkStatus_old_error(self):
-        @all_requests
-        def handler(url, request):
-            return {
-                "status_code": 200,
-                "content": {
-                    "completed": True, "application_url": "", "revision": None,
-                    "message": "You cannot use that reserved namespace.",
-                    "package_name": None,
-                    }}
-
-        status_url = "http://sca.example/dev/api/click-scan-complete/updown/1/";
-        with HTTMock(handler):
-            self.assertRaisesWithContent(
-                ScanFailedResponse, b"You cannot use that reserved namespace.",
-                self.client.checkStatus, status_url)
-
-    def test_checkStatus_old_success(self):
-        @all_requests
-        def handler(url, request):
-            return {
-                "status_code": 200,
-                "content": {
-                    "completed": True,
-                    "application_url": "http://sca.example/dev/click-apps/1/";,
-                    "revision": 1, "message": "", "package_name": "test",
-                    }}
-
-        status_url = "http://sca.example/dev/api/click-scan-complete/updown/1/";
-        with HTTMock(handler):
-            self.assertEqual(
-                ("http://sca.example/dev/click-apps/1/";, 1),
-                self.client.checkStatus(status_url))
-
-    def test_checkStatus_new_pending(self):
+    def test_checkStatus_pending(self):
         @all_requests
         def handler(url, request):
             return {
@@ -500,7 +419,7 @@
                 UploadNotScannedYetResponse, self.client.checkStatus,
                 status_url)
 
-    def test_checkStatus_new_error(self):
+    def test_checkStatus_error(self):
         @all_requests
         def handler(url, request):
             return {
@@ -521,7 +440,7 @@
                 b"You cannot use that reserved namespace.",
                 self.client.checkStatus, status_url)
 
-    def test_checkStatus_new_review_error(self):
+    def test_checkStatus_review_error(self):
         @all_requests
         def handler(url, request):
             return {
@@ -539,7 +458,7 @@
                 ScanFailedResponse, b"Review failed.",
                 self.client.checkStatus, status_url)
 
-    def test_checkStatus_new_complete(self):
+    def test_checkStatus_complete(self):
         @all_requests
         def handler(url, request):
             return {