Merge pull request #148 from Kinto/add-batch-debug-info

Add Batch debug information.

Author: Natim

CHANGELOG.rst

@@ -4,9 +4,17 @@ CHANGELOG
 This document describes changes between each past release.
 
 
-8.1.0 (unreleased)
+9.0.0 (unreleased)
 ==================
 
+**Breaking changes**
+
+- The client will fail a batch only when a 5XX error occurs (#148)
+
+**New Features**
+
+- Log all the batch responses (#148)
+- Log the request and the batch responses in debug (#148)
 - Allow reading responses from batch requests with the ``results()`` method. (#146)
 
 

kinto_http/batch.py

@@ -1,8 +1,13 @@
-from . import utils
+import json
+import logging
 from collections import defaultdict
 
 from kinto_http.exceptions import KintoException
 
+from . import utils
+
+logger = logging.getLogger(__name__)
+
 
 class BatchSession(object):
 
@@ -46,6 +51,7 @@ def _build_requests(self):
     def send(self):
         self._results = []
         requests = self._build_requests()
+        id_request = 0
         for chunk in utils.chunks(requests, self.batch_max_requests):
             kwargs = dict(method='POST',
                           endpoint=self.endpoints.get('batch'),
@@ -58,8 +64,25 @@ def send(self):
                     exception = KintoException(message)
                     exception.request = chunk[i]
                     exception.response = response
+
+                level = logging.WARN if status_code < 400 else logging.ERROR
+                message = response["body"].get("message", "")
+                logger.log(level, "Batch #{}: {} {} - {} {}".format(
+                    id_request, chunk[i]["method"], chunk[i]["path"],
+                    status_code, message))
+
+                # Full log in DEBUG mode
+                logger.debug("\nBatch #{}: \n\tRequest: {}\n\tResponse: {}\n".format(
+                    id_request, json.dumps(chunk[i]), json.dumps(response)))
+
+                # Raise in case of a 500
+                if status_code >= 500:
                     raise exception
+
+                id_request += 1
+
             self._results.append((resp, headers))
+
         return self._results
 
     def results(self):

kinto_http/tests/test_batch.py

@@ -96,10 +96,11 @@ def test_prefix_is_removed_from_batch_requests(self):
         _, kwargs1 = calls[0]
         assert kwargs1['payload']['requests'][0]['path'] == '/foobar'
 
-    def test_batch_raises_exception_as_soon_as_subrequest_fails(self):
+    def test_batch_raises_exception_as_soon_as_subrequest_fails_with_status_code_5xx(self):
         self.client.session.request.side_effect = [
             ({"responses": [
-                {"status": 404, "path": "/url2", "body": {}, "headers": {}}
+                {"status": 502, "path": "/url2", "body": {"message": "Host not reachable"},
+                 "headers": {}}
             ]}, mock.sentinel.headers),
             ({"responses": [
                 {"status": 200, "path": "/url1", "body": {}, "headers": {}},

kinto_http/tests/test_client.py

@@ -51,25 +51,43 @@ def test_batch_raises_exception(self):
             with self.client.batch(bucket='moz', collection='test') as batch:
                 batch.create_record(id=1234, data={'foo': 'bar'})
 
-    def test_batch_raises_exception_if_subrequest_failed(self):
+    def test_batch_raises_exception_if_subrequest_failed_with_code_5xx(self):
         error = {
             "errno": 121,
             "message": "This user cannot access this resource.",
-            "code": 403,
-            "error": "Forbidden"
+            "code": 500,
+            "error": "Server Internal Error"
         }
         self.session.request.side_effect = [
             ({"settings": {"batch_max_requests": 25}}, []),
             ({"responses": [
                 {"status": 200, "path": "/url1", "body": {}, "headers": {}},
-                {"status": 404, "path": "/url2", "body": error, "headers": {}}
+                {"status": 500, "path": "/url2", "body": error, "headers": {}}
             ]}, [])]
 
         with self.assertRaises(KintoException):
             with self.client.batch(bucket='moz', collection='test') as batch:
                 batch.create_record(id=1234, data={'foo': 'bar'})
                 batch.create_record(id=5678, data={'tutu': 'toto'})
 
+    def test_batch_dont_raise_exception_if_subrequest_failed_with_code_4xx(self):
+        error = {
+            "errno": 121,
+            "message": "Forbidden",
+            "code": 403,
+            "error": "This user cannot access this resource."
+        }
+        self.session.request.side_effect = [
+            ({"settings": {"batch_max_requests": 25}}, []),
+            ({"responses": [
+                {"status": 200, "path": "/url1", "body": {}, "headers": {}},
+                {"status": 403, "path": "/url2", "body": error, "headers": {}}
+            ]}, [])]
+
+        with self.client.batch(bucket='moz', collection='test') as batch:  # Do not raise
+            batch.create_record(id=1234, data={'foo': 'bar'})
+            batch.create_record(id=5678, data={'tutu': 'toto'})
+
     def test_batch_options_are_transmitted(self):
         settings = {"batch_max_requests": 25}
         self.session.request.side_effect = [({"settings": settings}, [])]