[GitHub] [beam] laraschmidt commented on a change in pull request #15540: [BEAM-12931] Allow for DoFn#getAllowedTimestampSkew() when checking the output timestamp

[GitBox <gi...@apache.org>]

laraschmidt commented on a change in pull request #15540:
URL: https://github.com/apache/beam/pull/15540#discussion_r714358223



##########
File path: runners/core-java/src/main/java/org/apache/beam/runners/core/SimpleDoFnRunner.java
##########
@@ -1190,12 +1190,12 @@ public Timer withOutputTimestamp(Instant outputTimestamp) {
      * </ul>
      */
     private void setAndVerifyOutputTimestamp() {
-
       if (outputTimestamp != null) {
         checkArgument(
-            !outputTimestamp.isBefore(elementInputTimestamp),

Review comment:
       So I ran the new validates runner tests against FnApiDoFnRunner. Turns out it was not properly accounting for DoFN skew when setting timers. But it wasn't doing any checking on timestamps at all for normal DoFn emit.
   
   I added checks for the normal DoFn case and allowed for skew in the timer case.




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org