lostluck commented on code in PR #31420:
URL: https://github.com/apache/beam/pull/31420#discussion_r1617959938


##########
sdks/go/pkg/beam/core/runtime/harness/harness.go:
##########
@@ -546,14 +546,15 @@ func (c *control) handleInstruction(ctx context.Context, 
req *fnpb.InstructionRe
                        }
                }
 
-               mons, pylds := monitoring(plan, store, 
c.runnerCapabilities[URNMonitoringInfoShortID])
+               mons, pylds, ConsumingReceivedData := monitoring(plan, store, 
c.runnerCapabilities[URNMonitoringInfoShortID])

Review Comment:
   Please make this lower case: ConsumingReceivedData. I'm pretty sure it'll 
trip a linter warning when we import it otherwise.



##########
sdks/java/core/src/main/java/org/apache/beam/sdk/fn/data/BeamFnDataInboundObserver.java:
##########
@@ -110,6 +114,10 @@ public void close() throws Exception {
     queue.cancel(CloseException.INSTANCE);
   }
 
+  public AtomicBoolean isConsumingReceivedData() {

Review Comment:
   I'd return the bool and make the get call here instead of making the caller 
use the AtomicBool. Encapsulate the implementation detail!



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