#1041 feat(observability): add failure monitoring signals#1640
#1041 feat(observability): add failure monitoring signals#1640Jacky-Pham wants to merge 3 commits into
Conversation
Jacky-Pham
changed the title
Jacky-Pham
requested review from
JazzarKarim,
dimak1 and
thorwolpert
as code owners
Jacky-Pham
changed the title
|
| email_info.interaction_uuid, | ||
| email_info.interaction, | ||
| err, | ||
| ) |
There was a problem hiding this comment.
The recipient list should be split in order to avoid the errors related to typos in email addresses
| email_info.registration_number, | ||
| email_info.interaction_uuid, | ||
| email_info.interaction, | ||
| err, |
There was a problem hiding this comment.
traceback was added to get the complete stack trace of the error. This helps in debugging
| metadata={ | ||
| "upload_source": "documents", | ||
| "document_type": "SUPPORTING_DOCUMENT", | ||
| "upload_step": "standalone_supporting_document", |
There was a problem hiding this comment.
I think we should add user metadata here as well. Just having these three attributes wont provide much info.
The document type should specify which one from the UI was selected.
| idempotency_key: str | None = None, | ||
| ): | ||
| """Create a queued interaction before publishing the email event.""" | ||
| # TODO: fix circular import issue |
There was a problem hiding this comment.
TODO left here. if the circular dependency has been resolved then these imports can be moved to the top
| interaction = session.get(CustomerInteraction, interaction_id) | ||
| if not interaction or not interaction.notify_reference: | ||
| return False | ||
| return "missing_reference" |
There was a problem hiding this comment.
QQ Whats the reason for changing the return type from boolean to random string values?
| try: | ||
| stale_sent_hours = int(os.getenv("STALE_SENT_HOURS", "24")) | ||
| stale_threshold = datetime.now(timezone.utc) - timedelta(hours=stale_sent_hours) | ||
| stale_stmt = select(CustomerInteraction.id).where( |
There was a problem hiding this comment.
there is another select statement below querying the same table... would it benefit to merge into one query and do comparison in here instead?
2 participants
Summary
Notes
Validation
Refs #1041