kennknowles commented on PR #40370: URL: https://github.com/apache/beam/pull/40370#issuecomment-6018973489
> > There are multiple changes here that need to be made separately IMO > > > > * changing the `leader_board.py` to set allowed lateness (I don't believe changing the trigger matters) > > * improving plumbing for a clock in trigger execution (this does look useful and good!) > > > > I believe just using allowed lateness with existing trigger should have the same impact on the integration test, FWIW. But I do think your other changes looks useful. > > Thanks @kennknowles, I can split these > > but allowed lateness is needed for the team windows. The test uses 1 minute windows and the event time is from before the job starts, so with lateness 0 those elements get dropped. The code already calculated 120 minutes, but it never passed that value to the window > > I don't think that fixes the user query though. leader_board_users is a global window, so those rows are not dropped for lateness. On the failing job all 500 messages were read, but the user score step only wrote 2 rows, and neither one was total_score 5000 so AfterCount(10) should have kept firing until that row showed up > > The processing time trigger is the same one the Java example uses as it fire after the data is already there so that pane is 5000 The clock changes are only for the unit tests, so I can move them to another PR Yes, I think we agree. I think the addition of allowed lateness is the thing that fixes the example. The change of trigger is neutral. I actually don't care too much either way about what trigger is used for the code demonstration. I was just thinking to keep the change to the minimum needed to fix it. -- 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]
