Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CHORE:fix JSONDecodeError for CIIM API request #1791

Merged
merged 4 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions etna/ciim/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ def fetch(
response = self.make_request(f"{self.base_url}/fetch", params=params)

# Convert the HTTP response to a Python dict
response_data = response.json()
response_data = self.decode_json_response(response)

# Convert the Python dict to a ResultList
result_list = self.resultlist_from_response(response_data)
Expand Down Expand Up @@ -399,7 +399,7 @@ def search(
response = self.make_request(f"{self.base_url}/search", params=params)

# Convert the HTTP response to a Python dict
response_data = response.json()
response_data = self.decode_json_response(response)

# Pull out the separate ES responses
bucket_counts_data, results_data = response_data["responses"]
Expand Down Expand Up @@ -457,7 +457,7 @@ def search_all(
response = self.make_request(f"{self.base_url}/searchAll", params=params)

# Convert the HTTP response to a Python dict
response_data = response.json()
response_data = self.decode_json_response(response)

# The API returns a series of ES 'responses', with results for each 'bucket'.
# Each of these responses is converted to it's own `ResultList`, and the collective
Expand Down Expand Up @@ -521,7 +521,7 @@ def search_unified(
response = self.make_request(f"{self.base_url}/searchUnified", params=params)

# Convert the HTTP response to a Python dict
response_data = response.json()
response_data = self.decode_json_response(response)

# The API returns a single ES response for this endpoint, which can be directly converted
# to a ResultList.
Expand Down Expand Up @@ -570,7 +570,7 @@ def fetch_all(
response = self.make_request(f"{self.base_url}/fetchAll", params=params)

# Convert the HTTP response to a Python dict
response_data = response.json()
response_data = self.decode_json_response(response)

# The API returns a single ES response for this endpoint, which can be directly converted
# to a ResultList.
Expand All @@ -597,6 +597,16 @@ def make_request(
self._raise_for_status(response)
return response

def decode_json_response(self, response):
"""Returns decoded JSON data using the built-in json decoder"""
try:
return response.json()
except ValueError as e:
# log exception value with response body
logger.error(f"{str(e)}:Response body:{response.text}")
# suppress double exception raising, keeping original exception available
raise Exception(e) from None

def _raise_for_status(self, response: requests.Response) -> None:
"""Raise custom error for any requests.HTTPError raised for a request.

Expand Down
54 changes: 54 additions & 0 deletions etna/ciim/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1174,3 +1174,57 @@ def test_valid_response(self):
response = self.records_client.fetch_all()
self.assertIsInstance(response, ResultList)
self.assertEqual(response.hits, ())



class DecodeJSONResponseTest(SimpleTestCase):
def setUp(self):
self.records_client = get_records_client()

@responses.activate
def test_decode_json_response_fetch(self):

responses.add(
responses.GET,
f"{settings.CLIENT_BASE_URL}/fetch",
status=204, # no content
body="",
content_type="application/json",
)

with self.assertLogs("etna.ciim.client", level="ERROR") as lc:
with self.assertRaisesMessage(
Exception, "Expecting value: line 1 column 1 (char 0)"
):
self.records_client.fetch()

self.assertIn(
"ERROR:etna.ciim.client:"
"Expecting value: line 1 column 1 (char 0):"
"Response body:",
lc.output,
)

@responses.activate
def test_decode_json_response_search(self):

responses.add(
responses.GET,
f"{settings.CLIENT_BASE_URL}/search",
status=204, # no content
body="",
content_type="application/json",
)

with self.assertLogs("etna.ciim.client", level="ERROR") as lc:
with self.assertRaisesMessage(
Exception, "Expecting value: line 1 column 1 (char 0)"
):
self.records_client.search()

self.assertIn(
"ERROR:etna.ciim.client:"
"Expecting value: line 1 column 1 (char 0):"
"Response body:",
lc.output,
)