Add record descriptor name to Elastic writer exception message#178
Add record descriptor name to Elastic writer exception message#178JSCU-CNI wants to merge 1 commit intofox-it:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #178 +/- ##
==========================================
+ Coverage 82.25% 83.50% +1.25%
==========================================
Files 34 34
Lines 3652 3608 -44
==========================================
+ Hits 3004 3013 +9
+ Misses 648 595 -53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
yunzheng
left a comment
There was a problem hiding this comment.
Oops, I remember reviewing this PR but it seems I forgot to add my actual review notes.
I was thinking maybe we can use Exception.add_note() here, that could be more clean. See also:
- https://docs.python.org/3/tutorial/errors.html#enriching-exceptions-with-notes
- https://daniel.feldroy.com/posts/til-2025-05-exception-add_note
However, this is a feature introduced in Python 3.11 but we still support Python 3.10 until EOL (2026-10). Luckily there is a backport package called exceptiongroup that we can use until we drop support.
Is this something you can explore for this PR? If not, i'm also happy to check.
That looks like a clean solution! As long as the message in the test is exposed we're happy.
Due to limited resources currently not unfortunately. |
This PR improves the exception message thrown when the Elastic adapter attempts to write documents that get rejected by the remote Elasticsearch server.