Restructure tim.opensearch bulk methods and retry deletes

[jonavellecuerdo]

Purpose and background context

The three bulk methods for indexing, updating, and deleting records from an OpenSearch index defined in tim.opensearch share a lot of the same code, but it was somewhat difficult to understand what was happening in its previous structure. The changes in this PR were intended to make the code easier to follow and set a clear path for integrating the retry solution for the bulk_delete method.

The main changes introduced in this PR is unpacking the tuple returned by streaming_bulk, followed by unpacking a key-value pair from a dictionary item. Applying these two changes simplified the way we access attributes in the response object (i.e., no more response[<index>][<operation]...). The commit message for afb61a5 describes the general control flow structure in more detail.

Note: When I originally started on this work, I was initially thinking I'd be introducing significant changes to the PR to remove the duplicated steps/code in bulk_* commands. However, the subtle differences for each method, such as:

• what key it updates in result_summary
  • bulk_index tracked ["created", "updated"]
  • bulk_update tracked ["updated", "skipped"]
  • bulk_delete tracked ["deleted"]
• whether to raise an error
  • bulk_index and bulk_update raise BulkOperationError when original response is non-loggable or non-retryable error and raises SingleOperationError or TimeoutError for failed retries
  • bulk_delete handles all errors, incrementing "errors" count
• what are considered loggable errors
  • bulk_index allows mapper_parsing_exception
  • bulk_update allows mapper_parsing_exception and document_missing_exception

make it challenging to abstract the method into smaller methods. Ultimately, I decided that implementing a private, sub-method was not worth it at this time and that a future larger-scale, refactoring of TIM would benefit from the individual, written out bulk_* method definitions!

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

See added unit test.

Includes new or updated dependencies?

NO

Changes expectations for external applications?

NO

What are the relevant tickets?

• https://mitlibraries.atlassian.net/browse/TIMX-639

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.

[Copilot]

Pull request overview

Refactors the OpenSearch bulk indexing/updating/deleting helpers in tim.opensearch to make streaming-bulk responses easier to interpret, and adds a retry path for bulk deletes on transport error 507, while aligning CLI error handling and updating tests accordingly.

Changes:

• Restructured bulk_index, bulk_update, and bulk_delete to unpack (ok, item) and parse response fields directly.
• Added retry-on-507 behavior for bulk_delete using execute_single_record_action.
• Standardized bulk error raising/handling around BulkOperationError and updated CLI/tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tim/opensearch.py	Refactors bulk methods’ response parsing; adds delete retry-on-507; updates single-record execution signature usage.
tim/errors.py	Adjusts BulkOperationError constructor parameter naming (action → operation).
tim/cli.py	Updates exception imports/handlers to catch BulkOperationError instead of BulkIndexingError.
tests/test_opensearch.py	Adds unit test coverage for bulk delete retry behavior on 507.
tests/test_cli.py	Updates tests to raise BulkOperationError with the new parameter name.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ehanson8]

Great work!