Skip to content

Use consistent error format in delta. #580

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.

Sign up for GitHub

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

Open
mfjarvis wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Use consistent error format in delta. #580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1b705c7
Use consistent error format in delta. Wait until service is ready to …
mfjarvis Jun 12, 2025
47e1f40
Increase number of attempts. Run check for sandbox too.
mfjarvis Jun 12, 2025
491873a
Merge branch 'master' into VED-000-consistent-error-format
mfjarvis Jun 12, 2025
2bcf49e
Merge branch 'master' into VED-000-consistent-error-format
mfjarvis Oct 7, 2025
d441a4a
Remove unused switch. Tidy up error handling in Converter init.
mfjarvis Oct 7, 2025
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
49 changes: 22 additions & 27 deletions delta_backend/src/converter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,58 +6,53 @@

class Converter:

def __init__(self, fhir_data, action_flag = ActionFlag.UPDATE, report_unexpected_exception=True):
def __init__(self, fhir_data, action_flag = ActionFlag.UPDATE):
self.converted = {}
self.error_records = []
self.action_flag = action_flag
self.report_unexpected_exception = report_unexpected_exception

try:
if not fhir_data:
raise ValueError("FHIR data is required for initialization.")

self.extractor = Extractor(fhir_data, self.report_unexpected_exception)
self.conversion_layout = ConversionLayout(self.extractor)
except Exception as e:
if report_unexpected_exception:
self._log_error(f"Initialization failed: [{e.__class__.__name__}] {e}")
raise

if not fhir_data:
raise ValueError("FHIR data is required for initialization.")

self.extractor = Extractor(fhir_data)
self.conversion_layout = ConversionLayout(self.extractor)

def run_conversion(self):
conversions = self.conversion_layout.get_conversion_layout()

for conversion in conversions:
self._convert_data(conversion)

self.error_records.extend(self.extractor.get_error_records())

# Add CONVERSION_ERRORS as the 35th field
self.converted["CONVERSION_ERRORS"] = self.error_records
return self.converted

def _convert_data(self, conversion: ConversionField):
flat_field = conversion.field_name_flat
try:
flat_field = conversion.field_name_flat

if flat_field == "ACTION_FLAG":
self.converted[flat_field] = self.action_flag
else:
converted = conversion.expression_rule()
if converted is not None:
self.converted[flat_field] = converted

except Exception as e:
self._log_error(f"Conversion error [{e.__class__.__name__}]: {e}", code=exception_messages.PARSING_ERROR)
self._log_error(
flat_field,
f"Conversion error [{e.__class__.__name__}]: {e}",
code=exception_messages.PARSING_ERROR
)
self.converted[flat_field] = ""

def _log_error(self,e,code=exception_messages.UNEXPECTED_EXCEPTION):
error_obj = {
def _log_error(self, field_name, e, code):
self.error_records.append({
"code": code,
"field": field_name,
"value": None,
"message": str(e)
}

if self.report_unexpected_exception:
self.error_records.append(error_obj)

})

def get_error_records(self):
return self.error_records
return self.error_records
28 changes: 12 additions & 16 deletions delta_backend/src/extractor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ class Extractor:
DATE_CONVERT_FORMAT = "%Y%m%d"
DEFAULT_POSTCODE = "ZZ99 3CZ"

def __init__(self, fhir_json_data, report_unexpected_exception = True):
def __init__(self, fhir_json_data):
self.fhir_json_data = json.loads(fhir_json_data, parse_float=decimal.Decimal) if isinstance(fhir_json_data, str) else fhir_json_data
self.report_unexpected_exception = report_unexpected_exception
self.error_records = []

def _get_patient(self):
Expand All @@ -40,8 +39,6 @@ def _get_valid_names(self, names, occurrence_time):

return names[0]



def _get_person_names(self):
occurrence_time = self._get_occurrence_date_time()
patient = self._get_patient()
Expand Down Expand Up @@ -165,18 +162,17 @@ def _get_site_information(self):
return site_code, site_code_type_uri

def _log_error(self, field_name, field_value, e, code=exception_messages.RECORD_CHECK_FAILED):
if self.report_unexpected_exception:
if isinstance(e, Exception):
message = exception_messages.MESSAGES[exception_messages.UNEXPECTED_EXCEPTION] % (e.__class__.__name__, str(e))
else:
message = str(e)

self.error_records.append({
"code": code,
"field": field_name,
"value": field_value,
"message": message
})
if isinstance(e, Exception):
message = exception_messages.MESSAGES[exception_messages.UNEXPECTED_EXCEPTION] % (e.__class__.__name__, str(e))
else:
message = str(e)

self.error_records.append({
"code": code,
"field": field_name,
"value": field_value,
"message": message
})

def _convert_date(self, field_name, date, format) -> str:
"""
Expand Down
70 changes: 13 additions & 57 deletions delta_backend/tests/test_convert.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def setUp(self):
# Start moto AWS mocks
self.mock = mock_aws()
self.mock.start()

"""Set up mock DynamoDB table."""
self.dynamodb_resource = boto3_resource("dynamodb", "eu-west-2")
self.table = self.dynamodb_resource.create_table(
Expand Down Expand Up @@ -74,7 +74,7 @@ def tearDown(self):
self.logger_exception_patcher.stop()
self.logger_info_patcher.stop()
self.mock_firehose_logger.stop()

self.mock.stop()

@staticmethod
Expand Down Expand Up @@ -126,34 +126,18 @@ def test_fhir_converter_json_direct_data(self):
json_data = json.dumps(ValuesForTests.json_data)

fhir_converter = Converter(json_data)
FlatFile = fhir_converter.run_conversion()
result = fhir_converter.run_conversion()

flatJSON = json.dumps(FlatFile)
result_str = json.dumps(result)
expected_imms_value = deepcopy(ValuesForTests.expected_imms2) # UPDATE is currently the default action-flag
expected_imms = json.dumps(expected_imms_value)
self.assertEqual(flatJSON, expected_imms)
self.assertEqual(result_str, expected_imms)

errorRecords = fhir_converter.get_error_records()
error_records = fhir_converter.get_error_records()

self.assertEqual(len(errorRecords), 0)

def test_fhir_converter_json_direct_data(self):
"""it should convert fhir json data to flat json"""
json_data = json.dumps(ValuesForTests.json_data)

fhir_converter = Converter(json_data)
FlatFile = fhir_converter.run_conversion()
self.assertEqual(len(error_records), 0)

flatJSON = json.dumps(FlatFile)
expected_imms_value = deepcopy(ValuesForTests.expected_imms2) # UPDATE is currently the default action-flag
expected_imms = json.dumps(expected_imms_value)
self.assertEqual(flatJSON, expected_imms)

errorRecords = fhir_converter.get_error_records()

self.assertEqual(len(errorRecords), 0)

def test_fhir_converter_json_error_scenario_reporting_on(self):
def test_fhir_converter_json_error_scenario(self):
"""it should convert fhir json data to flat json - error scenarios"""
error_test_cases = [ErrorValuesForTests.missing_json, ErrorValuesForTests.json_dob_error]

Expand All @@ -163,42 +147,18 @@ def test_fhir_converter_json_error_scenario_reporting_on(self):
fhir_converter = Converter(json_data)
fhir_converter.run_conversion()

errorRecords = fhir_converter.get_error_records()
error_records = fhir_converter.get_error_records()

# Check if bad data creates error records
self.assertTrue(len(errorRecords) > 0)

def test_fhir_converter_json_error_scenario_reporting_off(self):
"""it should convert fhir json data to flat json - error scenarios"""
error_test_cases = [ErrorValuesForTests.missing_json, ErrorValuesForTests.json_dob_error]

for test_case in error_test_cases:
json_data = json.dumps(test_case)

fhir_converter = Converter(json_data, report_unexpected_exception=False)
fhir_converter.run_conversion()

errorRecords = fhir_converter.get_error_records()
self.assertTrue(len(error_records) > 0)

# Check if bad data creates error records
self.assertTrue(len(errorRecords) == 0)

def test_fhir_converter_json_incorrect_data_scenario_reporting_on(self):
def test_fhir_converter_json_incorrect_data_scenario(self):
"""it should convert fhir json data to flat json - error scenarios"""

with self.assertRaises(ValueError):
fhir_converter = Converter(None)
errorRecords = fhir_converter.get_error_records()
self.assertTrue(len(errorRecords) > 0)


def test_fhir_converter_json_incorrect_data_scenario_reporting_off(self):
"""it should convert fhir json data to flat json - error scenarios"""

with self.assertRaises(ValueError):
fhir_converter = Converter(None, report_unexpected_exception=False)
errorRecords = fhir_converter.get_error_records()
self.assertTrue(len(errorRecords) == 0)
error_records = fhir_converter.get_error_records()
self.assertTrue(len(error_records) > 0)

def test_handler_imms_convert_to_flat_json(self):
"""Test that the Imms field contains the correct flat JSON data for CREATE, UPDATE, and DELETE operations."""
Expand Down Expand Up @@ -231,17 +191,13 @@ def test_handler_imms_convert_to_flat_json(self):
response
)

result = self.table.scan()
items = result.get("Items", [])
self.clear_table()

def clear_table(self):
scan = self.table.scan()
with self.table.batch_writer() as batch:
for item in scan.get("Items", []):
batch.delete_item(Key={"PK": item["PK"]})
result = self.table.scan()
items = result.get("Items", [])

if __name__ == "__main__":
unittest.main()