fix(datasets): do not double encode the data as json when saving an A…

…PIDataSet Signed-off-by: Florian Gaudin-Delrieu <[email protected]> Signed-off-by: Florian Gaudin-Delrieu <[email protected]>
kedro-org · Aug 10, 2023 · 25f0444 · 25f0444
1 parent ce957f1
commit 25f0444
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 9 deletions.
diff --git a/kedro-datasets/RELEASE.md b/kedro-datasets/RELEASE.md
@@ -2,6 +2,8 @@
 ## Major features and improvements
 
 ## Bug fixes and other changes
+* Fixed an issue on `api.APIDataSet` where the sent data was doubly converted to json
+  string (once by us and once by the `requests` library).
 
 ## Community contributions
 

diff --git a/kedro-datasets/kedro_datasets/api/api_dataset.py b/kedro-datasets/kedro_datasets/api/api_dataset.py
@@ -207,9 +207,9 @@ def _execute_save_with_chunks(
 
     def _execute_save_request(self, json_data: Any) -> requests.Response:
         try:
-            json_.loads(json_data)
+            self._request_args["json"] = json_.loads(json_data)
         except TypeError:
-            self._request_args["json"] = json_.dumps(json_data)
+            self._request_args["json"] = json_data
         try:
             response = requests.request(**self._request_args)
             response.raise_for_status()

diff --git a/kedro-datasets/tests/api/test_api_dataset.py b/kedro-datasets/tests/api/test_api_dataset.py
@@ -2,6 +2,7 @@
 import base64
 import json
 import socket
+from typing import Any
 
 import pytest
 import requests
@@ -23,7 +24,7 @@
 TEST_METHOD = "GET"
 TEST_HEADERS = {"key": "value"}
 
-TEST_SAVE_DATA = [json.dumps({"key1": "info1", "key2": "info2"})]
+TEST_SAVE_DATA = [{"key1": "info1", "key2": "info2"}]
 
 
 class TestAPIDataSet:
@@ -272,12 +273,22 @@ def test_socket_error(self, requests_mock):
             api_data_set.load()
 
     @pytest.mark.parametrize("method", POSSIBLE_METHODS)
-    def test_successful_save(self, requests_mock, method):
+    @pytest.mark.parametrize(
+        "data",
+        [TEST_SAVE_DATA, json.dumps(TEST_SAVE_DATA)],
+        ids=["data_as_dict", "data_as_json_string"],
+    )
+    def test_successful_save(self, requests_mock, method, data):
         """
         When we want to save some data on a server
         Given an APIDataSet class
-        Then check we get a response
+        Then check that the response is OK and the sent data is in the correct form.
         """
+
+        def json_callback(request: requests.Request, context: Any) -> dict:
+            """Callback that sends back the json."""
+            return request.json()
+
         if method in ["PUT", "POST"]:
             api_data_set = APIDataSet(
                 url=TEST_URL,
@@ -289,10 +300,12 @@ def test_successful_save(self, requests_mock, method):
                 TEST_URL_WITH_PARAMS,
                 headers=TEST_HEADERS,
                 status_code=requests.codes.ok,
+                json=json_callback,
             )
-            response = api_data_set._save(TEST_SAVE_DATA)
-
+            response = api_data_set._save(data)
             assert isinstance(response, requests.Response)
+            assert response.json() == TEST_SAVE_DATA
+
         elif method == "GET":
             api_data_set = APIDataSet(
                 url=TEST_URL,
@@ -315,6 +328,11 @@ def test_successful_save_with_json(self, requests_mock, save_methods):
         Given an APIDataSet class
         Then check we get a response
         """
+
+        def json_callback(request: requests.Request, context: Any) -> dict:
+            """Callback that sends back the json."""
+            return request.json()
+
         api_data_set = APIDataSet(
             url=TEST_URL,
             method=save_methods,
@@ -324,17 +342,20 @@ def test_successful_save_with_json(self, requests_mock, save_methods):
             save_methods,
             TEST_URL,
             headers=TEST_HEADERS,
-            text=json.dumps(TEST_JSON_RESPONSE_DATA),
+            json=json_callback,
         )
         response_list = api_data_set._save(TEST_SAVE_DATA)
-
         assert isinstance(response_list, requests.Response)
+        # check that the data was sent in the correct format
+        assert response_list.json() == TEST_SAVE_DATA
 
         response_dict = api_data_set._save({"item1": "key1"})
         assert isinstance(response_dict, requests.Response)
+        assert response_dict.json() == {"item1": "key1"}
 
         response_json = api_data_set._save(TEST_SAVE_DATA[0])
         assert isinstance(response_json, requests.Response)
+        assert response_json.json() == TEST_SAVE_DATA[0]
 
     @pytest.mark.parametrize("save_methods", SAVE_METHODS)
     def test_save_http_error(self, requests_mock, save_methods):