[Fetch Migrations] Use custom exceptions and other readability improvements #506
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kartg
requested review from
AndreKurait,
chelma,
gregschohn,
lewijacn,
mikaylathompson,
okhasawn and
sumobrian
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #506 +/- ##
=========================================
Coverage 76.06% 76.06%
Complexity 1356 1356
=========================================
Files 158 158
Lines 6011 6011
Branches 509 509
=========================================
Hits 4572 4572
Misses 1088 1088
Partials 351 351
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…gement Signed-off-by: Kartik Ganesh <[email protected]>
…dex_management.py and metadata_migration.py This commit adds custom exception classes contained within the exceptions.py file to improve readability in code and stack traces, and reduce the use of the generic RuntimeError class. Low-level GET requests now raise a RequestError, while index_management and metadata_migration each have their own higher-level exception classes. This commit also updates and corrects the unit tests to check for these exception classes, and ensure that the underlying cause is propogated. Signed-off-by: Kartik Ganesh <[email protected]>
…h to before they are read from the command line Signed-off-by: Kartik Ganesh <[email protected]>
kartg
force-pushed
the
fm-improvements
branch
from
c82268a to
810a1a4
Compare
gregschohn
approved these changes
Feb 13, 2024
gregschohn
pushed a commit
to gregschohn/opensearch-migrations
that referenced
this pull request
Feb 20, 2024
opensearch-project#506) This PR incorporates two pieces of prior feedback: 1. The "index_operations.py" file has been renamed to "index_management.py" to eliminate confusion on whether the operations in this file involve indexing of data (they don't). 2. Custom exceptions have been created for "index_management.py" and "metadata_migration.py" to use instead of the generic "RuntimeError" class to help disambiguate errors in the call stack. This change also includes a minor bugfix to the "showFetchMigrationCommand.sh" script to move default value declaration so that command-line arguments are parsed correctly. --------- Signed-off-by: Kartik Ganesh <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR incorporates two pieces of prior feedback from @gregschohn :
index_operations.pyfile has been renamed toindex_management.pyindex_management.pyandmetadata_migration.pyto use instead of the genericRuntimeErrorclass to help disambiguate errors in the call stackThis change also includes a minor bugfix to the
showFetchMigrationCommand.shscript to move default value declaration so that command-line arguments are parsed correctly.Issues Resolved
closes #491
Testing
Unit tests pass.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.