gianm commented on code in PR #18336:
URL: https://github.com/apache/druid/pull/18336#discussion_r2240598484


##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/shuffle/output/NilStageOutputReader.java:
##########
@@ -39,18 +34,11 @@ public class NilStageOutputReader implements 
StageOutputReader
 {
   public static final NilStageOutputReader INSTANCE = new 
NilStageOutputReader();
 
-  private static final byte[] EMPTY_FRAME_FILE;
-
-  static {
-    try {
-      final ByteArrayOutputStream baos = new ByteArrayOutputStream();
-      FrameFileWriter.open(Channels.newChannel(baos), null, 
ByteTracker.unboundedTracker()).close();
-      EMPTY_FRAME_FILE = baos.toByteArray();
-    }
-    catch (IOException e) {
-      throw new RuntimeException(e);
-    }
-  }
+  /**
+   * Frame file with no frames.
+   */
+  private static final byte[] EMPTY_FRAME_FILE =

Review Comment:
   > if we ever change the format for the frame file this change might get 
missed.
   This is a good thing, it will help us realize if we change the frame file 
format by accident ;P
   
   > Nit: Do you think it makes sense to have this byte array as part of 
`FrameFile`
   I dunno, so far this is the only place it was needed.



##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/shuffle/output/NilStageOutputReader.java:
##########
@@ -39,18 +34,11 @@ public class NilStageOutputReader implements 
StageOutputReader
 {
   public static final NilStageOutputReader INSTANCE = new 
NilStageOutputReader();
 
-  private static final byte[] EMPTY_FRAME_FILE;
-
-  static {
-    try {
-      final ByteArrayOutputStream baos = new ByteArrayOutputStream();
-      FrameFileWriter.open(Channels.newChannel(baos), null, 
ByteTracker.unboundedTracker()).close();
-      EMPTY_FRAME_FILE = baos.toByteArray();
-    }
-    catch (IOException e) {
-      throw new RuntimeException(e);
-    }
-  }
+  /**
+   * Frame file with no frames.
+   */
+  private static final byte[] EMPTY_FRAME_FILE =

Review Comment:
   > if we ever change the format for the frame file this change might get 
missed.
   
   This is a good thing, it will help us realize if we change the frame file 
format by accident ;P
   
   > Nit: Do you think it makes sense to have this byte array as part of 
`FrameFile`
   
   I dunno, so far this is the only place it was 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: [email protected]

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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to