gemini-code-assist[bot] commented on code in PR #38346:
URL: https://github.com/apache/beam/pull/38346#discussion_r3169883002


##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingModeExecutionContext.java:
##########
@@ -528,6 +529,11 @@ public Map<Long, Pair<Instant, Runnable>> flushState() {
           getWorkItem().getWorkToken(),
           activeReader);
       activeReader = null;
+    } else if (backlogBytes != UnboundedReader.BACKLOG_UNKNOWN && backlogBytes 
!= 1L) {

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The condition `backlogBytes != 1L` is used to ignore a legacy default value. 
However, if a user actually has a backlog of exactly 1 byte (or a value that 
casts to 1L), it will be incorrectly ignored. While likely negligible in most 
streaming scenarios, this magic number check is a bit brittle.



##########
sdks/java/core/src/main/java/org/apache/beam/sdk/io/Read.java:
##########
@@ -626,6 +627,22 @@ public ProcessContinuation processElement(
       return ProcessContinuation.resume();
     }
 
+    @GetSize
+    public double getSize(

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The `getSize` method throws a generic `Exception`. While the `@GetSize` 
annotation allows this, it is generally better practice to throw more specific 
exceptions, such as `IOException`, especially since `createReader` and 
`reader.start()` are the primary sources of failure 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: [email protected]

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

Reply via email to