Skip to content

Commit c3095a9

Browse files
raushan-skyflowraushan-skyflow
authored
SK-2142 return None on empty error response (#186)
* SK-2142 return None on empty error response
1 parent 8ec8c40 commit c3095a9

File tree

11 files changed

+48
-46
lines changed

11 files changed

+48
-46
lines changed

‎skyflow/utils/_utils.py‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,6 @@ def get_metrics():
211211
}
212212
return details_dic
213213

214-
215214
def parse_insert_response(api_response, continue_on_error):
216215
# Retrieve the headers and data from the API response
217216
api_response_headers = api_response.headers
@@ -239,13 +238,13 @@ def parse_insert_response(api_response, continue_on_error):
239238
error = {
240239
'request_index': idx,
241240
'request_id': request_id,
242-
'error': response['Body']['error']
241+
'error': response['Body']['error'],
242+
'http_code': response['Status'],
243243
}
244244
errors.append(error)
245245

246246
insert_response.inserted_fields = inserted_fields
247-
insert_response.errors = errors
248-
247+
insert_response.errors = errors if len(errors) > 0 else None
249248
else:
250249
for record in api_response_data.records:
251250
field_data = {
@@ -257,6 +256,7 @@ def parse_insert_response(api_response, continue_on_error):
257256

258257
inserted_fields.append(field_data)
259258
insert_response.inserted_fields = inserted_fields
259+
insert_response.errors = None
260260

261261
return insert_response
262262

@@ -275,21 +275,17 @@ def parse_delete_response(api_response: V1BulkDeleteRecordResponse):
275275
delete_response = DeleteResponse()
276276
deleted_ids = api_response.record_id_response
277277
delete_response.deleted_ids = deleted_ids
278-
delete_response.errors = []
278+
delete_response.errors = None
279279
return delete_response
280280

281-
282281
def parse_get_response(api_response: V1BulkGetRecordResponse):
283282
get_response = GetResponse()
284283
data = []
285-
errors = []
286284
for record in api_response.records:
287285
field_data = {field: value for field, value in record.fields.items()}
288286
data.append(field_data)
289287

290288
get_response.data = data
291-
get_response.errors = errors
292-
293289
return get_response
294290

295291
def parse_detokenize_response(api_response: HttpResponse[V1DetokenizeResponse]):
@@ -320,7 +316,7 @@ def parse_detokenize_response(api_response: HttpResponse[V1DetokenizeResponse]):
320316
errors = errors
321317
detokenize_response = DetokenizeResponse()
322318
detokenize_response.detokenized_fields = detokenized_fields
323-
detokenize_response.errors = errors
319+
detokenize_response.errors = errors if len(errors) > 0 else None
324320

325321
return detokenize_response
326322

@@ -357,7 +353,7 @@ def parse_invoke_connection_response(api_response: requests.Response):
357353
if 'x-request-id' in api_response.headers:
358354
metadata['request_id'] = api_response.headers['x-request-id']
359355

360-
return InvokeConnectionResponse(data=data, metadata=metadata)
356+
return InvokeConnectionResponse(data=data, metadata=metadata, errors=None)
361357
except Exception as e:
362358
raise SkyflowError(SkyflowMessages.Error.RESPONSE_NOT_JSON.value.format(content), status_code)
363359
except HTTPError:

‎skyflow/vault/connection/_invoke_connection_response.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
class InvokeConnectionResponse:
2-
def __init__(self, data=None, metadata=None):
2+
def __init__(self, data=None, metadata=None, errors=None):
33
self.data = data
44
self.metadata = metadata if metadata else {}
5+
self.errors = errors if errors else None
56

67
def __repr__(self):
7-
return f"ConnectionResponse('data'={self.data},'metadata'={self.metadata})"
8+
return f"ConnectionResponse('data'={self.data},'metadata'={self.metadata}), 'errors'={self.errors})"
89

910
def __str__(self):
1011
return self.__repr__()

‎skyflow/vault/controller/_detect.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ def output_to_dict_list(output):
151151
entities=entities,
152152
run_id=run_id_val,
153153
status=status_val,
154-
errors=[]
154+
errors=None
155155
)
156156

157157
def __get_token_format(self, request):

‎skyflow/vault/data/_insert_response.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
class InsertResponse:
22
def __init__(self, inserted_fields = None, errors=None):
3-
if errors is None:
4-
errors = list()
53
self.inserted_fields = inserted_fields
64
self.errors = errors
75

‎skyflow/vault/data/_query_response.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
class QueryResponse:
22
def __init__(self):
33
self.fields = []
4-
self.errors = []
4+
self.errors = None
55

66
def __repr__(self):
77
return f"QueryResponse(fields={self.fields}, errors={self.errors})"

‎skyflow/vault/data/_update_response.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
class UpdateResponse:
22
def __init__(self, updated_field = None, errors=None):
33
self.updated_field = updated_field
4-
self.errors = errors if errors is not None else []
4+
self.errors = errors
55

66
def __repr__(self):
77
return f"UpdateResponse(updated_field={self.updated_field}, errors={self.errors})"

‎skyflow/vault/detect/_deidentify_file_response.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def __init__(
1717
entities: list = None, # list of dicts with keys 'file' and 'extension'
1818
run_id: str = None,
1919
status: str = None,
20-
errors: list = [],
20+
errors: list = None,
2121
):
2222
self.file_base64 = file_base64
2323
self.file = File(file) if file else None

‎tests/utils/test__utils.py‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,12 @@ def test_parse_insert_response(self):
252252
result = parse_insert_response(api_response, continue_on_error=True)
253253
self.assertEqual(len(result.inserted_fields), 1)
254254
self.assertEqual(len(result.errors), 1)
255+
# Assert first successful record
256+
self.assertEqual(result.inserted_fields[0]["skyflow_id"], "id1")
257+
# Assert error record
258+
self.assertEqual(result.errors[0]["error"], TEST_ERROR_MESSAGE)
259+
self.assertEqual(result.errors[0]["http_code"], 400)
260+
self.assertEqual(result.errors[0]["request_id"], "12345")
255261

256262
def test_parse_insert_response_continue_on_error_false(self):
257263
mock_api_response = Mock()
@@ -270,7 +276,7 @@ def test_parse_insert_response_continue_on_error_false(self):
270276
]
271277
self.assertEqual(result.inserted_fields, expected_inserted_fields)
272278

273-
self.assertEqual(result.errors, [])
279+
self.assertEqual(result.errors, None)
274280

275281
def test_parse_update_record_response(self):
276282
api_response = Mock()
@@ -291,7 +297,7 @@ def test_parse_delete_response_successful(self):
291297
expected_deleted_ids = ["id_1", "id_2", "id_3"]
292298
self.assertEqual(result.deleted_ids, expected_deleted_ids)
293299

294-
self.assertEqual(result.errors, [])
300+
self.assertEqual(result.errors, None)
295301

296302
def test_parse_get_response_successful(self):
297303
mock_api_response = Mock()
@@ -310,7 +316,7 @@ def test_parse_get_response_successful(self):
310316
]
311317
self.assertEqual(result.data, expected_data)
312318

313-
self.assertEqual(result.errors, [])
319+
# self.assertEqual(result.errors, None)
314320

315321
def test_parse_detokenize_response_with_mixed_records(self):
316322
mock_api_response = Mock()
@@ -384,6 +390,7 @@ def test_parse_invoke_connection_response_successful(self, mock_response):
384390
self.assertIsInstance(result, InvokeConnectionResponse)
385391
self.assertEqual(result.data["key"], "value")
386392
self.assertEqual(result.metadata["request_id"], "1234")
393+
self.assertEqual(result.errors, None)
387394

388395
@patch("requests.Response")
389396
def test_parse_invoke_connection_response_json_decode_error(self, mock_response):

‎tests/vault/controller/test__connection.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ def test_invoke_success(self, mock_send):
5555
# Assertions for successful invocation
5656
expected_response = {
5757
'data': {"response": "success"},
58-
'metadata': {"request_id": "test-request-id"}
58+
'metadata': {"request_id": "test-request-id"},
59+
'errors': None
5960
}
6061
self.assertEqual(vars(response), expected_response)
6162
self.mock_vault_client.get_bearer_token.assert_called_once()

‎tests/vault/controller/test__detect.py‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ def test_deidentify_file_txt_success(self, mock_open, mock_basename, mock_base64
159159
word_count=1, char_count=1, size_in_kb=1,
160160
duration_in_seconds=None, page_count=None,
161161
slide_count=None, entities=[], run_id="runid123",
162-
status="SUCCESS", errors=[])) as mock_parse:
162+
status="SUCCESS", errors=None)) as mock_parse:
163163
result = self.detect.deidentify_file(req)
164164

165165
mock_validate.assert_called_once()
@@ -184,7 +184,7 @@ def test_deidentify_file_txt_success(self, mock_open, mock_basename, mock_base64
184184
self.assertIsNone(result.page_count)
185185
self.assertIsNone(result.slide_count)
186186
self.assertEqual(result.entities, [])
187-
self.assertEqual(result.errors, [])
187+
self.assertEqual(result.errors, None)
188188

189189
@patch("skyflow.vault.controller._detect.validate_deidentify_file_request")
190190
@patch("skyflow.vault.controller._detect.base64")
@@ -222,7 +222,7 @@ def test_deidentify_file_audio_success(self, mock_base64, mock_validate):
222222
word_count=1, char_count=1, size_in_kb=1,
223223
duration_in_seconds=1, page_count=None,
224224
slide_count=None, entities=[], run_id="runid456",
225-
status="SUCCESS", errors=[])) as mock_parse:
225+
status="SUCCESS", errors=None)) as mock_parse:
226226
result = self.detect.deidentify_file(req)
227227
mock_validate.assert_called_once()
228228
files_api.deidentify_audio.assert_called_once()
@@ -260,12 +260,11 @@ def test_get_detect_run_success(self, mock_validate):
260260
response.word_character_count = Mock(word_count=1, character_count=1)
261261
files_api.get_run.return_value = response
262262
with patch.object(self.detect, "_Detect__parse_deidentify_file_response",
263-
return_value=DeidentifyFileResponse(
264-
file="file", type="txt", extension="txt", word_count=1,
265-
char_count=1, size_in_kb=1, duration_in_seconds=None,
266-
page_count=None, slide_count=None, entities=[],
267-
run_id="runid789", status="SUCCESS",
268-
errors=[])) as mock_parse:
263+
return_value=DeidentifyFileResponse(file="file", type="txt", extension="txt", word_count=1,
264+
char_count=1, size_in_kb=1, duration_in_seconds=None,
265+
page_count=None, slide_count=None, entities=[],
266+
run_id="runid789", status="SUCCESS",
267+
errors=None)) as mock_parse:
269268
result = self.detect.get_detect_run(req)
270269
mock_validate.assert_called_once()
271270
files_api.get_run.assert_called_once()
@@ -678,7 +677,7 @@ def test_deidentify_file_using_file_path(self, mock_open, mock_basename, mock_ba
678677
entities=[],
679678
run_id="runid123",
680679
status="SUCCESS",
681-
errors=[]
680+
errors=None
682681
)) as mock_parse:
683682

684683
result = self.detect.deidentify_file(req)
@@ -709,4 +708,4 @@ def test_deidentify_file_using_file_path(self, mock_open, mock_basename, mock_ba
709708
self.assertIsNone(result.page_count)
710709
self.assertIsNone(result.slide_count)
711710
self.assertEqual(result.entities, [])
712-
self.assertEqual(result.errors, [])
711+
self.assertEqual(result.errors, None)

0 commit comments

Comments
 (0)