Remove AvailabilityForTablet #4728
Merged
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
- Delete AvailabiltyForTablet class. It is only being used
in 2 tests: ImportExportIT and TableOperationsIT.
- In both ImportExportIT and TableOperationsIT, replace
List<AvailabilityForTablet> objects with
Map<TabletId, TabletAvailability>.
- Fix typo in TableOperationsIT: verifyTabletAvailabilites ->
verifyTabletAvailabilities.
Resolves: apache#4706 "Remove AvailabilityForTablet from public API"
keith-turner
reviewed
Jul 5, 2024
| List<AvailabilityForTablet> expectedAvailability) throws TableNotFoundException { | ||
| public static void verifyTabletAvailabilities(AccumuloClient client, String tableName, | ||
| Range range, Map<TabletId,TabletAvailability> expectedAvailability) | ||
| throws TableNotFoundException { |
There was a problem hiding this comment.
With this new map, may be able to simplify the body of this method to the following.
Map<TabletId, TabletAvailability> seenAvailability = client.tableOperations()
.getTabletInformation(tableName, range).collect(Collectors.toMap(TabletInformation::getTabletId, TabletInformation::getTabletAvailability));
assertEquals(expectedAvailability, seenAvailability);There was a problem hiding this comment.
Makes sense, just updated.
keith-turner
approved these changes
Jul 5, 2024
- Streamline verifyTabletAvailabilities method in TableOperationsIT class by comparing two Maps instead of a Map and a List. Cut previous two Assertions down to one Assertion. Resolves: apache#4706 "Remove AvailabilityForTablet from public API"
- Correct formatting in TableOperationsIT which was previously causing workflow build to fail. Resolves: apache#4706 "Remove AvailabilityForTablet from public API"
|
@meatballspaghetti if this your first PR to Accumulo then feel free to make a PR to add yourself as a contributor on the People page. |
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.
This PR is meant to delete the
AvailabilityForTabletclass which was located in public API and only used by 2 integration tests:ImportExportITandTableOperationsIT. Upon deleting the class, its usages were replaced which maintained the original functionality. This PR also fixes a typo inTableOperationsIT.The following files have been modified:
core/src/main/java/org/apache/accumulo/core/client/admin/AvailabilityForTablet.java: Deleted.test/src/main/java/org/apache/accumulo/test/ImportExportIT.java: Replace usages ofList<AvailabilityForTablet>objects withMap<TabletId, TabletAvailability>.test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java: Replace usages ofList<AvailabilityForTablet>objects withMap<TabletId, TabletAvailability>. In doing so, changed the assertion on line 742: previously, was anassertEqualsassertion that compared theTabletIdfield of objects in expected/actual Lists; now, is anassertTrueassertion that checks if the (expected)Map<TabletId, TabletAvailability> expectedAvailabilitycontains the sameTabletId's as those in the (actual)List<TabletInformation> tabletInfo. Since there is an assertion (line 740) for checking that both structures have the same size, this change can serve the same purpose as the original assertion and ensures thatexpectedAvailabilityandtabletInfocontain equal values.verifyTabletAvailabilitieson line 735 ofTableOperationsIThas been simplified to compare two Maps instead of a Map and a List, resulting in only one Assertion needed.verifyTabletAvailabilites->verifyTabletAvailabilitiesin bothImportExportITandTableOperationsIT.Resolves: #4706 "Remove AvailabilityForTablet from public API"