You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "meatballspaghetti (via GitHub)" <gi...@apache.org> on 2024/07/05 17:43:44 UTC
[PR] Remove AvailabilityForTablet [accumulo]
meatballspaghetti opened a new pull request, #4728:
URL: https://github.com/apache/accumulo/pull/4728
This PR is meant to delete the `AvailabilityForTablet` class which was located in public API and only used by 2 integration tests: `ImportExportIT` and `TableOperationsIT`. Upon deleting the class, its usages were replaced which maintained the original functionality. This PR also fixes a typo in `TableOperationsIT`.
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 of `List<AvailabilityForTablet>` objects with `Map<TabletId, TabletAvailability>`.
- `test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java`: Replace usages of `List<AvailabilityForTablet>` objects with `Map<TabletId, TabletAvailability>`. In doing so, changed the assertion on line 742: previously, was an `assertEquals` assertion that compared the `TabletId` field of objects in expected/actual Lists; now, is an `assertTrue` assertion that checks if the (expected) `Map<TabletId, TabletAvailability> expectedAvailability` contains the same `TabletId`'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 that `expectedAvailability` and `tabletInfo` contain equal values.
- Fix all occurences of typo `verifyTabletAvailabilites` -> `verifyTabletAvailabilities` in both `ImportExportIT` and `TableOperationsIT`.
Resolves: #4706 "Remove AvailabilityForTablet from public API"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove AvailabilityForTablet [accumulo]
Posted by "meatballspaghetti (via GitHub)" <gi...@apache.org>.
meatballspaghetti commented on code in PR #4728:
URL: https://github.com/apache/accumulo/pull/4728#discussion_r1668804701
##########
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java:
##########
@@ -688,67 +688,68 @@ private void verifyTablesWithSplits(String tableName, Map<String,String> idMap,
tabletAvailability);
setExpectedTabletAvailability(expectedTabletAvailability, tableId, null, splitPts[2],
tabletAvailability);
- verifyTabletAvailabilites(tableName, new Range(), expectedTabletAvailability);
+ verifyTabletAvailabilities(tableName, new Range(), expectedTabletAvailability);
// verify individual tablets can be retrieved
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[0], null,
tabletAvailability);
- verifyTabletAvailabilites(tableName, new Range(null, new Text(splitPts[0])),
+ verifyTabletAvailabilities(tableName, new Range(null, new Text(splitPts[0])),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[1], splitPts[0],
tabletAvailability);
- verifyTabletAvailabilites(tableName,
+ verifyTabletAvailabilities(tableName,
new Range(new Text(splitPts[0]), false, new Text(splitPts[1]), true),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[2], splitPts[1],
tabletAvailability);
- verifyTabletAvailabilites(tableName,
+ verifyTabletAvailabilities(tableName,
new Range(new Text(splitPts[1]), false, new Text(splitPts[2]), true),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, null, splitPts[2],
tabletAvailability);
- verifyTabletAvailabilites(tableName, new Range(new Text(splitPts[2]), false, null, true),
+ verifyTabletAvailabilities(tableName, new Range(new Text(splitPts[2]), false, null, true),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[1], splitPts[0],
tabletAvailability);
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[2], splitPts[1],
tabletAvailability);
- verifyTabletAvailabilites(tableName,
+ verifyTabletAvailabilities(tableName,
new Range(new Text(splitPts[0]), false, new Text(splitPts[2]), true),
expectedTabletAvailability);
}
- public static void verifyTabletAvailabilites(String tableName, Range range,
- List<AvailabilityForTablet> expectedAvailability) throws TableNotFoundException {
- verifyTabletAvailabilites(accumuloClient, tableName, range, expectedAvailability);
+ public static void verifyTabletAvailabilities(String tableName, Range range,
+ Map<TabletId,TabletAvailability> expectedAvailability) throws TableNotFoundException {
+ verifyTabletAvailabilities(accumuloClient, tableName, range, expectedAvailability);
}
- public static void verifyTabletAvailabilites(AccumuloClient client, String tableName, Range range,
- List<AvailabilityForTablet> expectedAvailability) throws TableNotFoundException {
+ public static void verifyTabletAvailabilities(AccumuloClient client, String tableName,
+ Range range, Map<TabletId,TabletAvailability> expectedAvailability)
+ throws TableNotFoundException {
Review Comment:
Makes sense, just updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove AvailabilityForTablet [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4728:
URL: https://github.com/apache/accumulo/pull/4728#discussion_r1667088843
##########
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java:
##########
@@ -688,67 +688,68 @@ private void verifyTablesWithSplits(String tableName, Map<String,String> idMap,
tabletAvailability);
setExpectedTabletAvailability(expectedTabletAvailability, tableId, null, splitPts[2],
tabletAvailability);
- verifyTabletAvailabilites(tableName, new Range(), expectedTabletAvailability);
+ verifyTabletAvailabilities(tableName, new Range(), expectedTabletAvailability);
// verify individual tablets can be retrieved
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[0], null,
tabletAvailability);
- verifyTabletAvailabilites(tableName, new Range(null, new Text(splitPts[0])),
+ verifyTabletAvailabilities(tableName, new Range(null, new Text(splitPts[0])),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[1], splitPts[0],
tabletAvailability);
- verifyTabletAvailabilites(tableName,
+ verifyTabletAvailabilities(tableName,
new Range(new Text(splitPts[0]), false, new Text(splitPts[1]), true),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[2], splitPts[1],
tabletAvailability);
- verifyTabletAvailabilites(tableName,
+ verifyTabletAvailabilities(tableName,
new Range(new Text(splitPts[1]), false, new Text(splitPts[2]), true),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, null, splitPts[2],
tabletAvailability);
- verifyTabletAvailabilites(tableName, new Range(new Text(splitPts[2]), false, null, true),
+ verifyTabletAvailabilities(tableName, new Range(new Text(splitPts[2]), false, null, true),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[1], splitPts[0],
tabletAvailability);
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[2], splitPts[1],
tabletAvailability);
- verifyTabletAvailabilites(tableName,
+ verifyTabletAvailabilities(tableName,
new Range(new Text(splitPts[0]), false, new Text(splitPts[2]), true),
expectedTabletAvailability);
}
- public static void verifyTabletAvailabilites(String tableName, Range range,
- List<AvailabilityForTablet> expectedAvailability) throws TableNotFoundException {
- verifyTabletAvailabilites(accumuloClient, tableName, range, expectedAvailability);
+ public static void verifyTabletAvailabilities(String tableName, Range range,
+ Map<TabletId,TabletAvailability> expectedAvailability) throws TableNotFoundException {
+ verifyTabletAvailabilities(accumuloClient, tableName, range, expectedAvailability);
}
- public static void verifyTabletAvailabilites(AccumuloClient client, String tableName, Range range,
- List<AvailabilityForTablet> expectedAvailability) throws TableNotFoundException {
+ public static void verifyTabletAvailabilities(AccumuloClient client, String tableName,
+ Range range, Map<TabletId,TabletAvailability> expectedAvailability)
+ throws TableNotFoundException {
Review Comment:
With this new map, may be able to simplify the body of this method to the following.
```java
Map<TabletId, TabletAvailability> seenAvailability = client.tableOperations()
.getTabletInformation(tableName, range).collect(Collectors.toMap(TabletInformation::getTabletId, TabletInformation::getTabletAvailability));
assertEquals(expectedAvailability, seenAvailability);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove AvailabilityForTablet [accumulo]
Posted by "meatballspaghetti (via GitHub)" <gi...@apache.org>.
meatballspaghetti commented on code in PR #4728:
URL: https://github.com/apache/accumulo/pull/4728#discussion_r1668804701
##########
test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java:
##########
@@ -688,67 +688,68 @@ private void verifyTablesWithSplits(String tableName, Map<String,String> idMap,
tabletAvailability);
setExpectedTabletAvailability(expectedTabletAvailability, tableId, null, splitPts[2],
tabletAvailability);
- verifyTabletAvailabilites(tableName, new Range(), expectedTabletAvailability);
+ verifyTabletAvailabilities(tableName, new Range(), expectedTabletAvailability);
// verify individual tablets can be retrieved
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[0], null,
tabletAvailability);
- verifyTabletAvailabilites(tableName, new Range(null, new Text(splitPts[0])),
+ verifyTabletAvailabilities(tableName, new Range(null, new Text(splitPts[0])),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[1], splitPts[0],
tabletAvailability);
- verifyTabletAvailabilites(tableName,
+ verifyTabletAvailabilities(tableName,
new Range(new Text(splitPts[0]), false, new Text(splitPts[1]), true),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[2], splitPts[1],
tabletAvailability);
- verifyTabletAvailabilites(tableName,
+ verifyTabletAvailabilities(tableName,
new Range(new Text(splitPts[1]), false, new Text(splitPts[2]), true),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, null, splitPts[2],
tabletAvailability);
- verifyTabletAvailabilites(tableName, new Range(new Text(splitPts[2]), false, null, true),
+ verifyTabletAvailabilities(tableName, new Range(new Text(splitPts[2]), false, null, true),
expectedTabletAvailability);
expectedTabletAvailability.clear();
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[1], splitPts[0],
tabletAvailability);
setExpectedTabletAvailability(expectedTabletAvailability, tableId, splitPts[2], splitPts[1],
tabletAvailability);
- verifyTabletAvailabilites(tableName,
+ verifyTabletAvailabilities(tableName,
new Range(new Text(splitPts[0]), false, new Text(splitPts[2]), true),
expectedTabletAvailability);
}
- public static void verifyTabletAvailabilites(String tableName, Range range,
- List<AvailabilityForTablet> expectedAvailability) throws TableNotFoundException {
- verifyTabletAvailabilites(accumuloClient, tableName, range, expectedAvailability);
+ public static void verifyTabletAvailabilities(String tableName, Range range,
+ Map<TabletId,TabletAvailability> expectedAvailability) throws TableNotFoundException {
+ verifyTabletAvailabilities(accumuloClient, tableName, range, expectedAvailability);
}
- public static void verifyTabletAvailabilites(AccumuloClient client, String tableName, Range range,
- List<AvailabilityForTablet> expectedAvailability) throws TableNotFoundException {
+ public static void verifyTabletAvailabilities(AccumuloClient client, String tableName,
+ Range range, Map<TabletId,TabletAvailability> expectedAvailability)
+ throws TableNotFoundException {
Review Comment:
Makes sense, updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove AvailabilityForTablet [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on PR #4728:
URL: https://github.com/apache/accumulo/pull/4728#issuecomment-2221193494
@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](https://github.com/apache/accumulo-website/blob/main/pages/people.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove AvailabilityForTablet [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #4728:
URL: https://github.com/apache/accumulo/pull/4728
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org