[GitHub] [accumulo] dlmarion commented on a diff in pull request #2636: Update Apache parent POM, modernizer

[GitBox <gi...@apache.org>]

dlmarion commented on code in PR #2636:
URL: https://github.com/apache/accumulo/pull/2636#discussion_r852195468


##########
core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java:
##########
@@ -370,4 +372,15 @@ default void forEach(BiConsumer<? super Key,? super Value> keyValueConsumer) {
       keyValueConsumer.accept(entry.getKey(), entry.getValue());
     }
   }
+
+  /**
+   * Stream the Scanner results sequentially from this scanner's iterator
+   *

Review Comment:
   Should we add a note here that if the resulting stream cannot be used outside of a try-with-resources block that closes the Scanner and that if the stream is returned from a function without the Scanner being closed then `.onClose(scanner::close)` should be called before leaving the method?



##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/AmpleImpl.java:
##########
@@ -44,7 +44,7 @@ public TabletMetadata readTablet(KeyExtent extent, ReadConsistency readConsisten
     builder.readConsistency(readConsistency);
 
     try (TabletsMetadata tablets = builder.build()) {
-      return Iterables.getOnlyElement(tablets);
+      return tablets.stream().collect(onlyElement());

Review Comment:
   do we need `onClose(tablets::close)` here to close the underlying Scanner?



##########
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java:
##########
@@ -549,8 +548,7 @@ private Iterator<String> getOldCandidates(ServerContext context, String tableNam
     Range range = DeletesSection.getRange();
     Scanner scanner = context.createScanner(tableName, Authorizations.EMPTY);
     scanner.setRange(range);
-    return StreamSupport.stream(scanner.spliterator(), false)
-        .filter(entry -> !entry.getValue().equals(UPGRADED))
+    return scanner.stream().filter(entry -> !entry.getValue().equals(UPGRADED))
         .map(entry -> entry.getKey().getRow().toString().substring(OLD_DELETE_PREFIX.length()))

Review Comment:
   Do we need `.onClose(scanner::close)` here?



##########
server/base/src/main/java/org/apache/accumulo/server/metadata/ServerAmpleImpl.java:
##########
@@ -246,7 +244,7 @@ public Stream<ExternalCompactionFinalState> getExternalCompactionFinalStates() {
 
     scanner.setRange(ExternalCompactionSection.getRange());
     int pLen = ExternalCompactionSection.getRowPrefix().length();
-    return StreamSupport.stream(scanner.spliterator(), false)
+    return scanner.stream()
         .map(e -> ExternalCompactionFinalState.fromJson(
             ExternalCompactionId.of(e.getKey().getRowData().toString().substring(pLen)),
             e.getValue().toString()));

Review Comment:
   do we need `.onClose(scanner::close)` here?



-- 
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