keith-turner commented on code in PR #3418:
URL: https://github.com/apache/accumulo/pull/3418#discussion_r1213781360


##########
core/src/main/java/org/apache/accumulo/core/client/rfile/RFile.java:
##########
@@ -76,6 +77,15 @@ public interface InputArguments {
      * @return this
      */
     ScannerFSOptions from(String... files);
+
+    /**
+     * Specify RFiles to read from. When multiple are specified the {@link 
Scanner} constructed will
+     * present a merged view.
+     *
+     * @param files one or more RFiles to read.
+     * @return this
+     */
+    ScannerFSOptions from(FencedRfile... files);

Review Comment:
   Needs a since tag in javadoc



##########
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileSource.java:
##########
@@ -29,10 +32,12 @@
 public class RFileSource {
   private final InputStream in;
   private final long len;
+  private final Range range;
 
-  public RFileSource(InputStream in, long len) {
-    this.in = in;
+  public RFileSource(InputStream in, long len, Range range) {

Review Comment:
   This is a breaking API change.  Need to add a new overloaded constructors 
that take a range.



##########
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScannerBuilder.java:
##########
@@ -150,4 +162,24 @@ public ScannerOptions withBounds(Range range) {
     this.opts.bounds = range;
     return this;
   }
+
+  // UnreferencedTabletFile is not allowed here
+  // as part of the public API so we need a new object
+  public static class FencedRfile {

Review Comment:
   If in public API it needs a since tag.
   
   The class RFileScannerBuilder is package private. I think it contains 
implementation code behind the public API, but the type RFileScannerBuilder is 
not in the public API. This FencedRfile class should probably move into RFile 
or RFile.InputArguments.
   
   Can not wait until we can use Java's new record type for classes like this.



##########
core/src/main/java/org/apache/accumulo/core/metadata/AbstractTabletFile.java:
##########
@@ -53,4 +56,21 @@ public Path getPath() {
     return path;
   }
 
+  /**
+   * @return The range of the TabletFile
+   *
+   * @since 3.1.0

Review Comment:
   This is not in public API so since tag is not needed.



##########
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileSource.java:
##########
@@ -42,4 +47,8 @@ public InputStream getInputStream() {
   public long getLength() {
     return len;
   }
+
+  public Range getRange() {

Review Comment:
   needs since tag in javadoc.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to