Fix error parsing when uploading file (#4)

* Fix error parsing when uploading file

* Fix response handling when downloading a file

* formatted merged code

Author: jitsuineccles

archivist/archivist.py

@@ -124,6 +124,7 @@ def get_file(self, subpath, identity, fd, *, headers=None):
         subpath: e.g. v2 or iam/v1
         identity: e.g. assets/xxxxxxxxxxxxxxxxxxxxxxxxxxxx`
         fd: an iterable representing a file (usually from open())
+            the file must be opened in binary mode
         """
         response = requests.get(
             SEP.join((self.url, ROOT, subpath, identity)),
@@ -140,7 +141,7 @@ def get_file(self, subpath, identity, fd, *, headers=None):
             if chunk:
                 fd.write(chunk)
 
-        return response.json()
+        return response
 
     def post(self, path, request, *, headers=None):
         """
@@ -168,6 +169,7 @@ def post_file(self, path, fd, mtype):
 
         path: e.g. v1/blobs
         fd: an iterable representing a file (usually from open())
+            the file must be opened in binary mode
         mtype: mime tiype (image/jpg)
         """
 

archivist/attachments.py

@@ -23,13 +23,14 @@ def upload(self, fd, *, mtype="image/jpg"):
         )
 
     def download(self, identity, fd):
-        """docstring"""
-        return Attachment(
-            **self._archivist.get_file(
-                ATTACHMENTS_SUBPATH,
-                identity,
-                fd,
-            )
+        """
+        Note that returns the response as the body will be consumed by the
+        fd iterator
+        """
+        return self._archivist.get_file(
+            ATTACHMENTS_SUBPATH,
+            identity,
+            fd,
         )
 
 

archivist/errors.py

@@ -63,8 +63,14 @@ def __identity(response):
         req = response.request
         body = getattr(req, "body", None)
         if body:
-            body = json.loads(body)
-            identity = body.get("identity", "unknown")
+            # when uploading a file the body attribute is a
+            # MultiPartEncoder
+            try:
+                body = json.loads(body)
+            except TypeError:
+                pass
+            else:
+                identity = body.get("identity", "unknown")
 
     return identity
 

unittests/testerrors.py

@@ -10,6 +10,8 @@
 
 from unittest import TestCase
 
+from requests_toolbelt.multipart.encoder import MultipartEncoder
+
 from archivist.errors import (
     parse_response,
     Archivist4xxError,
@@ -37,6 +39,7 @@ def test_errors_200(self):
         """
         response = MockResponse(200)
         error = parse_response(response)
+
         self.assertEqual(
             error,
             None,
@@ -49,6 +52,7 @@ def test_errors_300(self):
         """
         response = MockResponse(300)
         error = parse_response(response)
+
         self.assertEqual(
             error,
             None,
@@ -61,12 +65,14 @@ def test_errors_400(self):
         """
         response = MockResponse(400, error="some error")
         error = parse_response(response)
+
         self.assertIsNotNone(
             error,
             msg="error should not be None",
         )
         with self.assertRaises(ArchivistBadRequestError) as ex:
             raise error
+
         self.assertEqual(
             str(ex.exception),
             '{"error": "some error"} (400)',
@@ -79,12 +85,14 @@ def test_errors_401(self):
         """
         response = MockResponse(401, error="some error")
         error = parse_response(response)
+
         self.assertIsNotNone(
             error,
             msg="error should not be None",
         )
         with self.assertRaises(ArchivistUnauthenticatedError) as ex:
             raise error
+
         self.assertEqual(
             str(ex.exception),
             '{"error": "some error"} (401)',
@@ -97,12 +105,46 @@ def test_errors_403(self):
         """
         response = MockResponse(403, error="some error")
         error = parse_response(response)
+
         self.assertIsNotNone(
             error,
             msg="error should not be None",
         )
         with self.assertRaises(ArchivistForbiddenError) as ex:
             raise error
+
+        self.assertEqual(
+            str(ex.exception),
+            '{"error": "some error"} (403)',
+        )
+
+    def test_errors_403_multipartencoder(self):
+        """
+        Test errors
+        """
+
+        class Object:
+            pass
+
+        request = Object()
+        request.body = MultipartEncoder(
+            fields={
+                "file": ("filename", "filecontents", "image/jpg"),
+            }
+        )
+        response = MockResponse(
+            403,
+            request=request,
+            error="some error",
+        )
+        error = parse_response(response)
+        self.assertIsNotNone(
+            error,
+            msg="error should not be None",
+        )
+        with self.assertRaises(ArchivistForbiddenError) as ex:
+            raise error
+
         self.assertEqual(
             str(ex.exception),
             '{"error": "some error"} (403)',