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


##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/CompactionMetadata.java:
##########
@@ -93,10 +95,31 @@ public FateId getFateId() {
     return fateId;
   }
 
+  private static class InputFile {
+    String path;
+    String startRow;
+    String endRow;
+
+    private static InputFile from(StoredTabletFile stf) {
+      InputFile i = new InputFile();
+      i.path = stf.getPath().toString();
+      Range r = stf.getRange();
+      i.startRow = r.getStartKey() == null ? "null" : 
r.getStartKey().toString();

Review Comment:
   May not want to pass data through toString as that could corrupt binary 
data.  It would be nice if we could use the internal class and machinery in 
StoredTabletFile here, but as @cshannon mentioned that would break 
encapsulation.  Encapsulation is nice and I would not want to see 
StoredTabletFile internals in my ide when doing completion, we could break 
encapsulation in a limited way possibly by doing the following.
   
    * Move StoredTabletFile and CompactionMetadata into the same package.  They 
are almost in the same package, one is in` o.a.a.c.metadata` and the other is 
in `o.a.a.c.metadata.schema`.
    * Change StoredTabletFile to make TabletFileCqMetadataGson package private 
and add some package private static methods to go to from 
TabletFileCqMetadataGson<->StoredTabletFile
    * In CompactionMetadata.GSonData use TabletFileCqMetadataGson 
   
   This way most of the Accumulo code can not see 
StoredTabletFile.TabletFileCqMetadataGson and we still get a nice human 
readable encoding maybe.



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