Skip to content

Conversation

hakbailey
Copy link
Contributor

@hakbailey hakbailey commented Mar 3, 2023

What does this PR do?

Updates bulk-indexing to skip error responses only for the known mapper parsing error, and otherwise raise an exception that stops the process. Also updates dependencies and config.

Helpful background context

We were getting some errors indicating a problem within OpenSearch, and in cases like that we don't want to continue indexing as though nothing is wrong.

Includes new or updated dependencies?

YES

What are the relevant tickets?

Developer

  • All new ENV is documented in README (or there is none)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

* Update Python version
* Update OpenSearch Docker image in README
* Minor configuration updates
* Bump boto3 from 1.26.66 to 1.26.83
* Bump opensearch-py from 2.1.1 to 2.2.0
* Bump sentry-sdk from 1.15.0 to 1.16.0
* Bump mypy from 1.0.0 to 1.0.1
* Bump pydocstyle from 6.1.1 to 6.3.0
Why these changes are being introduced:
Previously when bulk-indexing returned an error from Opensearch, we
logged the error, skipped the record, and continued indexing. We still
want to do that for the known error around mapper parsing (since that is
a data quality/transformation issue, not an Opensearch issue) but for
all other errors we want to stop the process with an exception for
further investigation.

How this addresses that need:
* Adds BulkIndexingError to errors module.
* During bulk indexing, checks for the known error we want to skip and
  otherwise raises the new exception.
* Updates tests and fixtures to reflect changes.
* Minor documentation update in cli module.

Side effects of this change:
If we hit any errors other than the occasional expected mapping error,
the Step Function will fail. This is good.
@hakbailey hakbailey requested review from ehanson8 and JPrevost March 3, 2023 17:17
Copy link
Contributor

@ehanson8 ehanson8 left a comment

Choose a reason for hiding this comment

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

Smart approach 👍

@hakbailey hakbailey merged commit d266bc8 into main Mar 3, 2023
@hakbailey hakbailey deleted the stop-indexing-for-unexpected-errors branch March 3, 2023 21:22
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