Skip to content

Add AftersynchronizedProcessing Time as continuation trigger#35913

Merged
kennknowles merged 6 commits intoapache:masterfrom
tarun-google:continuation_trigger_parity
Aug 25, 2025
Merged

Add AftersynchronizedProcessing Time as continuation trigger#35913
kennknowles merged 6 commits intoapache:masterfrom
tarun-google:continuation_trigger_parity

Conversation

@tarun-google
Copy link
Contributor

Fixes #34212

In extension to previously discarded work : #14060

  1. Added get_continuation_trigger in parity with Java code base and implemented it in all the triggers.
  2. Added the internal AfterSynchronizedProcessing time, as a continuation trigger for the AfterProcessingTime trigger which always fire and move the pipeline forward. This is not in parity with Java code with waits for synchronized time
  3. Added get_continuation_trigger impl for all the non time based triggers in parity with java code.

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

@tarun-google tarun-google changed the title Add Asynchronized Processing Time as continuation trigger Add AftersynchronizedProcessing Time as continuation trigger Aug 19, 2025
@tarun-google tarun-google marked this pull request as ready for review August 20, 2025 14:54
@tarun-google
Copy link
Contributor Author

@tarun-google
Copy link
Contributor Author

R: @ahmedabu98 @kennknowles

@github-actions
Copy link
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

@derrickaw
Copy link
Collaborator

Hi @tarun-google - please fix the lint issues. Looks like some imports are incorrectly sorted. Thanks!

Copy link
Contributor

@ahmedabu98 ahmedabu98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM, but I think @kennknowles should do a pass too

@liferoad liferoad requested a review from kennknowles August 20, 2025 20:16
Copy link
Member

@kennknowles kennknowles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I did a quick pass to check against Java just to make sure they are consistent. (not worrying too much about if they are ideal, just as long as they match it is the right thing to do )

@tarun-google tarun-google requested a review from derrickaw August 21, 2025 19:56
@derrickaw
Copy link
Collaborator

Run Python_Transforms PreCommit 3.12

@kennknowles
Copy link
Member

Please get the tests green (either by re-running or fixing real issues) and I'll merge.

@kennknowles
Copy link
Member

If there are new very-flaky tests, please file bugs for them. There may already be bugs filed based on other workflows being flaky, but figuring out which tests are the flakes would be very useful.

@derrickaw
Copy link
Collaborator

Run Python_Examples PreCommit 3.10

@derrickaw
Copy link
Collaborator

Run Python_Integration PreCommit 3.9

@tarun-google
Copy link
Contributor Author

Thanks @kennknowles & @derrickaw for triggering 👍 I will follow up the tests

@tarun-google
Copy link
Contributor Author

Run Yaml_Xlang_Direct PreCommit

@tarun-google
Copy link
Contributor Author

Run Python_Runners PreCommit 3.9

@tarun-google
Copy link
Contributor Author

All green Yay 🥇

@kennknowles kennknowles merged commit 47eb9da into apache:master Aug 25, 2025
100 of 105 checks passed
@liferoad
Copy link
Contributor

Runner V2 does not support this yet. Revert this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Task]: Add Python AfterSynchronizedProcessingTime trigger and add an Iceberg CDC streaming read test

5 participants