Skip to content

refactor: rename of error code and messages related to empty entities… #187

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

Merged
georgeRobertson merged 1 commit into from
Oct 13, 2026
Merged

refactor: rename of error code and messages related to empty entities… #187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions docs/user_guidance/entity_relationships.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ There is also the functionality to customise errors related to either missing pa
- `missing_parent_id_error_message`: the error message to display if a record is rejected as it has no valid parent record
- `no_valid_records_error_code`: the error code to display if parent records are removed due to no valid children in a mandatory field
- `no_valid_records_error_message`: the error message to display if parent records are removed due to no valid children in a mandatory field
- `empty_entity_error_code`: the error code to display if a __mandatory__ entity has any records post filtering
- `empty_entity_error_message`: the error message to display if a __mandatory__ entity has any records post filtering
- `empty_entity_post_br_error_code`: the error code to display if a __mandatory__ entity has any records post filtering
- `empty_entity_post_br_error_message`: the error message to display if a __mandatory__ entity has any records post filtering

!!! note
Specifying root entities is __optional__. Root entities will be inferred based on their absence.
Expand Down
4 changes: 2 additions & 2 deletions src/dve/core_engine/configuration/v1/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ class _LinkageConfig(BaseModel):
"Records removed due to no valid parent record"
)
"""The error code to emit if the entity contains records that are orphaned by parent record rejections""" # pylint: disable=C0301
empty_entity_error_code: ErrorCode = "EmptyEntity"
empty_entity_post_br_error_code: ErrorCode = "EmptyEntity"
"""The error code to emit if a mandatory entity has no valid remaining records"""
empty_entity_error_message: ErrorMessage = "no valid records remaining"
empty_entity_post_br_error_message: ErrorMessage = "no valid records remaining"
"""The error message to emit if a mandatory entity has no valid remaining records"""

@model_validator(mode="after")
Expand Down
8 changes: 4 additions & 4 deletions src/dve/core_engine/configuration/v1/hierarchy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
missing_parent_id_error_message: Optional[ErrorMessage] = (
"Records removed due to no valid parent record"
)
empty_entity_error_code: ErrorCode = "EmptyEntity"
empty_entity_error_message: ErrorMessage = "no valid records remaining"
empty_entity_post_br_error_code: ErrorCode = "EmptyEntity"
empty_entity_post_br_error_message: ErrorMessage = "no valid records remaining"

@model_validator(mode="after")
def validate_empty_error_details(self):
Expand All @@ -36,8 +36,8 @@
non mandatory nodes/entities
"""
if not self.mandatory:
self.empty_entity_error_code = None
self.empty_entity_error_message = None
self.empty_entity_post_br_error_code = None
self.empty_entity_post_br_error_message = None
return self

def get_descendents(self) -> list["HierarchyNode"]:
Expand All @@ -52,7 +52,7 @@
"""Recursively list all names of descendents of the node"""
return [node.entity_name for node in self.get_descendents()]

def get_node(self, entity_name: str) -> Union["HierarchyNode", None]:

Check warning on line 55 in src/dve/core_engine/configuration/v1/hierarchy.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a union type expression for this type hint. 


See more on https://sonarcloud.io/project/issues?id=NHSDigital_data-validation-engine&issues=AaE1fBe2McUvPv1Dbj7P&open=AaE1fBe2McUvPv1Dbj7P&pullRequest=186
"""Recursively search for node and return if found"""
node = None
if self.entity_name == entity_name:
Expand Down
4 changes: 2 additions & 2 deletions src/dve/pipeline/pipeline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,10 +581,10 @@
entity=entity_name,
record=None,
error_location=entity_name,
error_message=node.empty_entity_error_message,
error_message=node.empty_entity_post_br_error_message,
failure_type="submission",
error_type="submission",
error_code=node.empty_entity_error_code,
error_code=node.empty_entity_post_br_error_code,
category="Empty entity",
)
)
Expand Down Expand Up @@ -707,12 +707,12 @@

entity_issues: dict[EntityName, bool] = {
entity: any(
val
for val in (
orph_issues_1.get(entity, False),
grp_issues_1.get(entity, False),
orph_issues_2.get(entity, False),
)

Check warning on line 715 in src/dve/pipeline/pipeline.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace this comprehension with passing the iterable to the collection constructor call 


See more on https://sonarcloud.io/project/issues?id=NHSDigital_data-validation-engine&issues=AaE1fBl0McUvPv1Dbj7b&open=AaE1fBl0McUvPv1Dbj7b&pullRequest=186
)
for entity in orph_issues_1.keys()
}
Expand Down Expand Up @@ -886,7 +886,7 @@
aggregates_df.write_parquet(output_uri)
return output_uri

@lru_cache() # noqa: B019

Check warning on line 889 in src/dve/pipeline/pipeline.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove "@lru_cache"/"@cache" from this instance method; the cache retains "self" and can leak memory. 


See more on https://sonarcloud.io/project/issues?id=NHSDigital_data-validation-engine&issues=AaE1fBl0McUvPv1Dbj7c&open=AaE1fBl0McUvPv1Dbj7c&pullRequest=186
def _get_error_dataframes(self, submission_id: str):
if not self.processed_files_path:
raise AttributeError("processed files path not provided")
Expand Down Expand Up @@ -1020,17 +1020,17 @@
pool: Executor,
processed: Iterable[tuple[SubmissionInfo, Optional[SubmissionStatus]]] = tuple(),
failed_file_transformation: Iterable[tuple[SubmissionInfo, SubmissionStatus]] = tuple(),
) -> list[
tuple[SubmissionInfo, SubmissionStatus, Union[None, SubmissionStatisticsRecord], URI]
]:

Check warning on line 1025 in src/dve/pipeline/pipeline.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a union type expression for this type hint. 


See more on https://sonarcloud.io/project/issues?id=NHSDigital_data-validation-engine&issues=AaE1fBl0McUvPv1Dbj7d&open=AaE1fBl0McUvPv1Dbj7d&pullRequest=186
"""Step to produce error reports
takes processed files and files that failed file transformation
"""
self._logger.info("Starting error reports service")
futures: list[tuple[SubmissionInfo, SubmissionStatus, Future]] = []
reports: list[
tuple[SubmissionInfo, SubmissionStatus, Union[None, SubmissionStatisticsRecord], URI]
] = []

Check warning on line 1033 in src/dve/pipeline/pipeline.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a union type expression for this type hint. 


See more on https://sonarcloud.io/project/issues?id=NHSDigital_data-validation-engine&issues=AaE1fBl0McUvPv1Dbj7e&open=AaE1fBl0McUvPv1Dbj7e&pullRequest=186
failed_processing: list[tuple[SubmissionInfo, SubmissionStatus]] = []

for info, status in processed:
Expand Down Expand Up @@ -1095,7 +1095,7 @@
submission_files = self._get_submission_files_for_run()

# parse files to parquet order doesn't matter
with ThreadPoolExecutor(max_workers=max_workers) as pool:

Check warning on line 1098 in src/dve/pipeline/pipeline.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Combine these nested "with" statements into a single "with" with multiple contexts. 


See more on https://sonarcloud.io/project/issues?id=NHSDigital_data-validation-engine&issues=AaE1fBl0McUvPv1Dbj7f&open=AaE1fBl0McUvPv1Dbj7f&pullRequest=186
with self._audit_tables:
audited, _ = self.audit_received_file_step(pool, submission_files)
# what should we do with files that fail auditing - likely to be an internal matter -
Expand Down
32 changes: 16 additions & 16 deletions tests/test_core_engine/test_hierarchy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,8 @@ def test_linkage_config_load():
"no_valid_records_error_message": "parent record removed as no valid child records",
"missing_parent_id_error_code": null,
"missing_parent_id_error_message": null,
"empty_entity_error_code": null,
"empty_entity_error_message": null,
"empty_entity_post_br_error_code": null,
"empty_entity_post_br_error_message": null,
"children": {
"ds_003": {
"parent_entity": "ds_001",
Expand All @@ -282,8 +282,8 @@ def test_linkage_config_load():
"no_valid_records_error_message": "parent record removed as no valid child records",
"missing_parent_id_error_code": "DS003NoParent",
"missing_parent_id_error_message": "record removed as no parent",
"empty_entity_error_code": null,
"empty_entity_error_message": null,
"empty_entity_post_br_error_code": null,
"empty_entity_post_br_error_message": null,
"children": {}
},
"ds_101": {
Expand All @@ -296,8 +296,8 @@ def test_linkage_config_load():
"no_valid_records_error_message": "{{ ds_001_id }} removed as no valid ds_101 records",
"missing_parent_id_error_code": "DS101NoParent",
"missing_parent_id_error_message": "record removed as no parent",
"empty_entity_error_code": null,
"empty_entity_error_message": null,
"empty_entity_post_br_error_code": null,
"empty_entity_post_br_error_message": null,
"children": {
"ds_201": {
"parent_entity": "ds_101",
Expand All @@ -309,8 +309,8 @@ def test_linkage_config_load():
"no_valid_records_error_message": "parent record removed as no valid child records",
"missing_parent_id_error_code": "DS201NoParent",
"missing_parent_id_error_message": "record removed as no parent",
"empty_entity_error_code": null,
"empty_entity_error_message": null,
"empty_entity_post_br_error_code": null,
"empty_entity_post_br_error_message": null,
"children": {
"ds_202": {
"parent_entity": "ds_201",
Expand All @@ -322,8 +322,8 @@ def test_linkage_config_load():
"no_valid_records_error_message": "parent record removed as no valid child records",
"missing_parent_id_error_code": "MissingParentRecord",
"missing_parent_id_error_message": "Records removed due to no valid parent record",
"empty_entity_error_code": "EmptyEntity",
"empty_entity_error_message": "no valid records remaining",
"empty_entity_post_br_error_code": "EmptyEntity",
"empty_entity_post_br_error_message": "no valid records remaining",
"children": {}
}
}
Expand Down Expand Up @@ -351,8 +351,8 @@ def test_linkage_config_load():
"no_valid_records_error_message": "{{ ds_001_id }} removed as no valid ds_101 records",
"missing_parent_id_error_code": "DS101NoParent",
"missing_parent_id_error_message": "record removed as no parent",
"empty_entity_error_code": null,
"empty_entity_error_message": null,
"empty_entity_post_br_error_code": null,
"empty_entity_post_br_error_message": null,
"children": {
"ds_201": {
"parent_entity": "ds_101",
Expand All @@ -364,8 +364,8 @@ def test_linkage_config_load():
"no_valid_records_error_message": "parent record removed as no valid child records",
"missing_parent_id_error_code": "DS201NoParent",
"missing_parent_id_error_message": "record removed as no parent",
"empty_entity_error_code": null,
"empty_entity_error_message": null,
"empty_entity_post_br_error_code": null,
"empty_entity_post_br_error_message": null,
"children": {
"ds_202": {
"parent_entity": "ds_201",
Expand All @@ -377,8 +377,8 @@ def test_linkage_config_load():
"no_valid_records_error_message": "parent record removed as no valid child records",
"missing_parent_id_error_code": "MissingParentRecord",
"missing_parent_id_error_message": "Records removed due to no valid parent record",
"empty_entity_error_code": "EmptyEntity",
"empty_entity_error_message": "no valid records remaining",
"empty_entity_post_br_error_code": "EmptyEntity",
"empty_entity_post_br_error_message": "no valid records remaining",
"children": {}
}
}
Expand Down
12 changes: 6 additions & 6 deletions tests/testdata/flights/flights.dischema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@
"country": {
"is_root_entity": true,
"mandatory": true,
"empty_entity_error_code": "NoValidCountries",
"empty_entity_error_message": "File Rejected - There are no valid country records"
"empty_entity_post_br_error_code": "NoValidCountries",
"empty_entity_post_br_error_message": "File Rejected - There are no valid country records"
},
"airport": {
"parent_entity": "country",
Expand All @@ -174,8 +174,8 @@
"missing_parent_id_error_message": "Record rejected - No valid country id found for airport",
"no_valid_records_error_code": "CountryHasNoAirport",
"no_valid_records_error_message": "Group rejected - Unable to find any valid airports",
"empty_entity_error_code": "NoValidAirports",
"empty_entity_error_message": "File Rejected - There are no valid airport records"
"empty_entity_post_br_error_code": "NoValidAirports",
"empty_entity_post_br_error_message": "File Rejected - There are no valid airport records"
},
"staff": {
"parent_entity": "airport",
Expand All @@ -187,8 +187,8 @@
"missing_parent_id_error_message": "Record rejected - No valid airport id found for staff. Airport ID = {{ airport_id }}, Staff ID = {{ staff_id }}",
"no_valid_records_error_code": "AirportHasNoStaff",
"no_valid_records_error_message": "Group rejected - Airport has no valid staff. Airport ID = {{ airport_id }}",
"empty_entity_error_code": "NoValidStaff",
"empty_entity_error_message": "File Rejected - There are no valid staff records"
"empty_entity_post_br_error_code": "NoValidStaff",
"empty_entity_post_br_error_message": "File Rejected - There are no valid staff records"
},
"flights": {
"parent_entity": "airport",
Expand Down
8 changes: 4 additions & 4 deletions tests/testdata/flights/flights_spark.dischema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@
"missing_parent_id_error_message": "Record rejected - No valid country id found for airport",
"no_valid_records_error_code": "CountryHasNoAirport",
"no_valid_records_error_message": "Group rejected - Unable to find any valid airports",
"empty_entity_error_code": "NoValidAirports",
"empty_entity_error_message": "File Rejected - There are no valid airport records"
"empty_entity_post_br_error_code": "NoValidAirports",
"empty_entity_post_br_error_message": "File Rejected - There are no valid airport records"
},
"staff": {
"parent_entity": "airport",
Expand All @@ -187,8 +187,8 @@
"missing_parent_id_error_message": "Record rejected - No valid airport id found for staff. Airport ID = {{ airport_id }}, Staff ID = {{ staff_id }}",
"no_valid_records_error_code": "AirportHasNoStaff",
"no_valid_records_error_message": "Group rejected - Airport has no valid staff. Airport ID = {{ airport_id }}",
"empty_entity_error_code": "NoValidStaff",
"empty_entity_error_message": "File Rejected - There are no valid staff records"
"empty_entity_post_br_error_code": "NoValidStaff",
"empty_entity_post_br_error_message": "File Rejected - There are no valid staff records"
},
"flights": {
"parent_entity": "airport",
Expand Down
Loading