je-ik commented on code in PR #23302:
URL: https://github.com/apache/beam/pull/23302#discussion_r976621771


##########
sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/PeriodicSequence.java:
##########
@@ -178,9 +182,22 @@ public RestrictionTracker<OffsetRange, Long> 
newTracker(@Restriction OffsetRange
       return new OutputRangeTracker(restriction);
     }
 
+    @GetInitialWatermarkEstimatorState
+    public Instant getInitialWatermarkState() {
+      return BoundedWindow.TIMESTAMP_MIN_VALUE;

Review Comment:
   This should be fine, the initial state should be used only when the pipeline 
is first run, the watermark is expected to be on its minimum value by then.



##########
sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/PeriodicSequence.java:
##########
@@ -178,9 +182,22 @@ public RestrictionTracker<OffsetRange, Long> 
newTracker(@Restriction OffsetRange
       return new OutputRangeTracker(restriction);
     }
 
+    @GetInitialWatermarkEstimatorState
+    public Instant getInitialWatermarkState() {
+      return BoundedWindow.TIMESTAMP_MIN_VALUE;
+    }
+
+    @NewWatermarkEstimator
+    public WatermarkEstimator<Instant> newWatermarkEstimator(
+        @WatermarkEstimatorState Instant state) {
+
+      return new WatermarkEstimators.Manual(state);

Review Comment:
   I prefer to add newline after method declaration that spans multiple lines. 
Looks like it is pretty common in the Beam's codebase, so I'd prefer to keep it 
as it is.



##########
sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/PeriodicSequence.java:
##########
@@ -178,9 +182,22 @@ public RestrictionTracker<OffsetRange, Long> 
newTracker(@Restriction OffsetRange
       return new OutputRangeTracker(restriction);
     }
 
+    @GetInitialWatermarkEstimatorState

Review Comment:
   I think that is no problem. The important part is that @ProcessElement 
returns `ProcessContinuation`.



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