You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/12/15 18:59:32 UTC
[GitHub] [accumulo] DomGarguilo opened a new pull request, #3129: Replace copied HostAndPort with guava
DomGarguilo opened a new pull request, #3129:
URL: https://github.com/apache/accumulo/pull/3129
The class `HostAndPort` that is currently in the code was copied from an older version of guava when it was still in beta. Now that the version of guava that we use no longer has it marked as beta, it seems like a good idea to use that directly.
Most of these changes are simply import statement changes. Aside from those changes:
* `core/util/HostAndPort.java` and `core/util/HostAndPortTest.java` were deleted.
* a `compareTo` was added to our version of `HostAndPort` that is not in guava but as it was only used in one place, `VerifyTabletAssignments`, the comparator was just copied for use into that file.
--
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
[GitHub] [accumulo] DomGarguilo commented on pull request #3129: Replace copied HostAndPort with guava
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on PR #3129:
URL: https://github.com/apache/accumulo/pull/3129#issuecomment-1354932330
> I saw you removed the HostAndPort test but I would think we should keep it and update it because one of the main things that was tested was the comparator functionality which doesn't exist in the Guava version so that still needs to be tested.
Good point. Maybe the test should be renamed to HostAndPortComparatorTest or something like that.
--
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
[GitHub] [accumulo] DomGarguilo commented on a diff in pull request #3129: Replace copied HostAndPort with guava
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #3129:
URL: https://github.com/apache/accumulo/pull/3129#discussion_r1050936066
##########
server/base/src/main/java/org/apache/accumulo/server/util/VerifyTabletAssignments.java:
##########
@@ -95,7 +97,10 @@ private static void checkTable(final ClientContext context, final boolean verbos
final HashSet<KeyExtent> failures = new HashSet<>();
- Map<HostAndPort,List<KeyExtent>> extentsPerServer = new TreeMap<>();
+ Comparator<HostAndPort> comparator = Comparator.nullsFirst(
Review Comment:
Addressed in 6495b62
--
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
[GitHub] [accumulo] DomGarguilo commented on pull request #3129: Replace copied HostAndPort with guava
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on PR #3129:
URL: https://github.com/apache/accumulo/pull/3129#issuecomment-1355173177
Added the test back and created a util class for the comparator in 6495b62a99f206daace7ff4e62ec12eb69238c5c
--
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
[GitHub] [accumulo] cshannon commented on a diff in pull request #3129: Replace copied HostAndPort with guava
Posted by GitBox <gi...@apache.org>.
cshannon commented on code in PR #3129:
URL: https://github.com/apache/accumulo/pull/3129#discussion_r1050657191
##########
server/base/src/main/java/org/apache/accumulo/server/util/VerifyTabletAssignments.java:
##########
@@ -95,7 +97,10 @@ private static void checkTable(final ClientContext context, final boolean verbos
final HashSet<KeyExtent> failures = new HashSet<>();
- Map<HostAndPort,List<KeyExtent>> extentsPerServer = new TreeMap<>();
+ Comparator<HostAndPort> comparator = Comparator.nullsFirst(
Review Comment:
You should be able to make this static and create it just one time in the class and reuse the comparator just like we were doing originally in the copied version. It might be better to even have it in a public utility class somewhere so we could re-use it elsewhere if needed.
--
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
[GitHub] [accumulo] DomGarguilo merged pull request #3129: Replace copied HostAndPort with guava
Posted by GitBox <gi...@apache.org>.
DomGarguilo merged PR #3129:
URL: https://github.com/apache/accumulo/pull/3129
--
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