Skip to content

Conversation

@GeoWill
Copy link
Contributor

@GeoWill GeoWill commented Jan 13, 2026

No description provided.

This commit deals with two types of error.
First it adds a sentry log when there is a file not found on s3.
Second it catches any exceptions raised when calling the code which gets
the boundary review response. It then logs this to sentry, but still
returns a response. This seemed better than raising a 5xx in this
situation, where a client might want the rest of the information in the
response, even if boundary change failed.
@GeoWill GeoWill marked this pull request as ready for review February 3, 2026 13:14
@GeoWill GeoWill changed the title WiP models for boundary changes endpoint. Boundary changes endpoint. Feb 3, 2026
data = self.get_data_for_uprn()
return self.query_to_dict(data)

def data_quality_check(self, postcode_df):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now happens inside every request/response cycle that looks for static data.
I don't think it will add much, but it feels like the wrong place for it.

Would it be better to try and add some checks to the statemachine that run at the end of each run and do data quality assurance.

except Exception as ex:
# Log to sentry, but don't fail to return response
sentry_sdk.capture_exception(ex)
resp["boundary_reviews"] = "Error fetching boundary reviews"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is different to what we did for recall petitions, but feels like it's probably more useful. i.e. say we had a problem in the API response, but still return all the other data.

Comment on lines +108 to +109
except FileNotFoundError:
resp["boundary_reviews"] = None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In these cases we should be logging the error to sentry, but not telling the user there was an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants