Merge ~maxiberta/snapstore-client:improve-error-handling-2 into snapstore-client:master

[Maximiliano Bertacchini]

Improve handling of push exceptions

[Wouter van Bommel (woutervb)]

lgtm, minor comment would be that newer code in this project uses f strings

review: Approve

[Otto Co-Pilot (otto-copilot)]

Running landing tests failed
https://jenkins.ols.canonical.com/online-services/job/snapstore-client/44/

diff --git a/store_admin/exceptions.py b/store_admin/exceptions.py
index 0f1f71f..7883a99 100644
--- a/store_admin/exceptions.py
+++ b/store_admin/exceptions.py
@@ -28,6 +28,10 @@ class InvalidStoreURL(ClientError):
     fmt = "Invalid store url: {message}."
 
 
+class PushException(ClientError):
+    fmt = "Error uploading data: {message}."
+
+
 class StoreCommunicationError(ClientError):
     fmt = "Connection error with the store using: {message}."
 
diff --git a/store_admin/logic/push.py b/store_admin/logic/push.py
index 274cf98..b612f14 100644
--- a/store_admin/logic/push.py
+++ b/store_admin/logic/push.py
@@ -25,10 +25,6 @@ logger = logging.getLogger(__name__)
 USERNAME = "admin"
 
 
-class pushException(Exception):
-    pass
-
-
 class ChannelMapExistsException(Exception):
     pass
 
@@ -63,7 +59,7 @@ def _push_ident(store, password, downloaded_map):
         auth=requests.auth.HTTPBasicAuth(USERNAME, password),
     )
     if response.status_code != 200:
-        raise pushException(
+        raise exceptions.PushException(
             "Failure in pushing to snapident: {}".format(response.content)
         )
 
@@ -109,7 +105,7 @@ def _push_revs(store, password, downloaded_map):
         )
         # 409 / Conflict - already exists
         if revs_response.status_code not in [201, 409]:
-            raise pushException(
+            raise exceptions.PushException(
                 "Failure to push revisions to snaprevs: {}".format(
                     revs_response.content
                 )
@@ -138,7 +134,7 @@ def _push_channelmap(store, password, downloaded_map, force_push=False):
         auth=requests.auth.HTTPBasicAuth(USERNAME, password),
     )
     if existing.status_code != 200:
-        raise pushException("Error retrieving current channelmap")
+        raise exceptions.PushException("Error retrieving current channelmap")
     if existing.json().get("channelmaps") and not force_push:
         raise ChannelMapExistsException("Channel map already exists.")
 
@@ -173,7 +169,7 @@ def _push_channelmap(store, password, downloaded_map, force_push=False):
             auth=requests.auth.HTTPBasicAuth(USERNAME, password),
         )
         if map_response.status_code != 200:
-            raise pushException("Error pushing channel map")
+            raise exceptions.PushException("Error pushing channel map")
 
 
 def _load_assertion_file(assert_path):
@@ -224,7 +220,7 @@ def _add_assertion_to_service(store, password, list_assertions):
             auth=requests.auth.HTTPBasicAuth(USERNAME, password),
         )
         if response.status_code != 201:
-            raise pushException(
+            raise exceptions.PushException(
                 "Failed to push: {} - {}".format(response.status_code, response.content)
             )
 
@@ -239,8 +235,9 @@ def _push_assertions(store, password, assert_path):
     # push the assertions to the internal snap-assertion-service
     try:
         _add_assertion_to_service(store, password, current_asserts)
-    except pushException:
-        raise pushException("Assertion file: {}".format(assert_path))
+    except exceptions.PushException:
+        logger.error("Assertion file: %s", assert_path)
+        raise
 
 
 def _push_file_to_nginx_cache(store, password, snap_path, snap_id, revision):
@@ -254,8 +251,7 @@ def _push_file_to_nginx_cache(store, password, snap_path, snap_id, revision):
         auth=requests.auth.HTTPBasicAuth(USERNAME, password),
     )
     if response.status_code != 200:
-        print(response.status_code)
-        raise pushException("Failed to push file to proxy cache")
+        raise exceptions.PushException("Failed to push file to proxy cache")
 
 
 def _push(store, tar_file, password, force_channel_map=False):
diff --git a/store_admin/logic/tests/test_push.py b/store_admin/logic/tests/test_push.py
index c694646..432be19 100644
--- a/store_admin/logic/tests/test_push.py
+++ b/store_admin/logic/tests/test_push.py
@@ -5,9 +5,9 @@ from urllib.parse import urljoin
 
 import responses
 
+from store_admin.exceptions import PushException
 from store_admin.logic.push import (
     ChannelMapExistsException,
-    pushException,
     _add_assertion_to_service,
     _split_assertions,
     _push_channelmap,
@@ -44,7 +44,7 @@ class Testpush:
         ident_url = urljoin(self.default_gw_url, "/snaps/update")
         responses.add("POST", ident_url, status=500)
 
-        pytest.raises(pushException, _push_ident, self.store, "test", self.snap_map)
+        pytest.raises(PushException, _push_ident, self.store, "test", self.snap_map)
 
     @responses.activate
     def test_push_revs(self):
@@ -64,14 +64,14 @@ class Testpush:
         revs_url = urljoin(self.default_gw_url, "/revisions/create")
         responses.add("POST", revs_url, status=302)
 
-        pytest.raises(pushException, _push_revs, self.store, "test", self.snap_map)
+        pytest.raises(PushException, _push_revs, self.store, "test", self.snap_map)
 
     @responses.activate
     def test_push_revs_failed(self):
         revs_url = urljoin(self.default_gw_url, "/revisions/create")
         responses.add("POST", revs_url, status=500)
 
-        pytest.raises(pushException, _push_revs, self.store, "test", self.snap_map)
+        pytest.raises(PushException, _push_revs, self.store, "test", self.snap_map)
 
     @responses.activate
     def test_push_map(self):
@@ -107,7 +107,7 @@ class Testpush:
         responses.add("POST", update_url, status=500)
 
         pytest.raises(
-            pushException, _push_channelmap, self.store, "test", self.snap_map
+            PushException, _push_channelmap, self.store, "test", self.snap_map
         )
 
     @responses.activate
@@ -174,7 +174,7 @@ class Testpush:
 
         split_assertions = _split_assertions(self.snap_assert)
         pytest.raises(
-            pushException,
+            PushException,
             _add_assertion_to_service,
             self.store,
             "test",
@@ -188,7 +188,7 @@ class Testpush:
 
         split_assertions = _split_assertions(self.snap_assert)
         pytest.raises(
-            pushException,
+            PushException,
             _add_assertion_to_service,
             self.store,
             "test",