Skip to content

Retries resulting in allowed errors are handled#391

Merged
jonavellecuerdo merged 1 commit into
mainfrom
TIMX-639-retry-raises-allowed-exceptions
Jul 14, 2026
Merged

Retries resulting in allowed errors are handled#391
jonavellecuerdo merged 1 commit into
mainfrom
TIMX-639-retry-raises-allowed-exceptions

Conversation

@jonavellecuerdo

@jonavellecuerdo jonavellecuerdo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Purpose and background context

If a retry results in an allowed error, these errors should be handled as well. It's important to note that during a retry of a failed action, TIM uses the single-document APIs (i.e., the index, update, and delete methods), which will raise exceptions whereas streaming_bulk(raise_on_error=False) did not. The retry decorator has been updated to re-raise NotFoundError and RequestError caused by mapper parsing exception so they can be handled by the corresponding tim.opensearch.bulk_* methods.

How can a reviewer manually see the effects of these changes?

Review added unit tests.

Includes new or updated dependencies?

NO

Changes expectations for external applications?

NO

What are the relevant tickets?

Code review

  • Code review best practices are documented here and you are encouraged to have a constructive dialogue with your reviewers about their preferences and expectations.

Why these changes are being introduced:
* If a retry results in an allowed error, these errors should be handled as well.
It's important to note that during a retry of a failed action, TIM uses the
single-document APIs (i.e., the `index`, `update`, and `delete` methods), which
will raise exceptions whereas streaming_bulk(raise_on_error=False) did not.
The retry decorator has been updated to re-raise `NotFoundError` and `RequestError`
caused by mapper parsing exception so they can be handled by the
corresponding `tim.opensearch.bulk_*` methods.

How this addresses that need:
* `tim.opensearch.bulk_index` handles `RequestError` caused by mapper parsing
exception raised from retries
* `tim.opensearch.bulk_update` handles `NotFoundError` caused by missing
documents and `RequestError` caused by mapper parsing exception raised from retries
* `tim.opensearch.bulk_delete` handles `NotFoundError` caused by missing
documents

Side effects of this change:
* None

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/TIMX-639
@jonavellecuerdo
jonavellecuerdo force-pushed the TIMX-639-retry-raises-allowed-exceptions branch from 6f094f9 to 540ae97 Compare July 14, 2026 17:48
@jonavellecuerdo
jonavellecuerdo marked this pull request as ready for review July 14, 2026 17:51
@jonavellecuerdo
jonavellecuerdo requested a review from a team as a code owner July 14, 2026 17:51
@jonavellecuerdo
jonavellecuerdo requested a review from ehanson8 July 14, 2026 17:52

@ehanson8 ehanson8 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One optional suggestion but this is a clean solution!

Comment thread tests/test_helpers.py
Comment thread tim/helpers.py
Comment thread tim/helpers.py
@jonavellecuerdo
jonavellecuerdo merged commit 4046fc1 into main Jul 14, 2026
5 checks passed
@jonavellecuerdo
jonavellecuerdo deleted the TIMX-639-retry-raises-allowed-exceptions branch July 14, 2026 19:38
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