GeezerVison is showing github.com — [python] Check a paused output connector's record counter on every host by blp · Pull Request #7608 · feldera/feldera
Conversation
blp
commented
Oct 23, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Draft: merge only after #7607 and feldera/cloud#2123 are merged and deployed where the multihost Python tests run. Before that, this test hangs with more than one host.
This reverts bbc9569 and a2cc16c (from #7582). They limited the check that a paused output connector reports input processed after the pause to one host, because the connector's total_processed_input_records counted only the input of its own host, and they added waits on steps for more than one host in its place.
With #7607 and feldera/cloud#2123, the coordinator reports that counter for the input of all of the hosts. So the check works with any number of hosts again, and it gives the connector time to show that it stays quiet, so the waits on steps are not needed.
test_orchestration.py is now identical to the version before bbc9569. Tested: test_output_connector_orchestration passes (2 passed) with 2 hosts on a local k3d cluster built from feldera/cloud#2123, which includes #7607. Still to do before merging: both changes deployed where the CI python-multihost suite runs.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This reverts bbc9569 and a2cc16c. They limited the check that a
paused output connector reports input processed after the pause to one
host, because the connector's `total_processed_input_records` counted
only the input of its own host, and they added waits on steps for more
than one host in its place.
Now the coordinator reports that counter for the input of all of the
hosts, so the check works with any number of hosts again, and it gives
the connector time to show that it stays quiet, so the waits on steps
are not needed.
This needs a coordinator and pipelines that have the multihost
committed records change.
|
Review summary. This PR changes only a test. It reverts the single-host guard and the multihost step waits in test_output_connector_orchestration, so the total_processed_input_records > processed_at_pause check now runs on every host count.
| Check |
Result |
pre-commit run --files python/tests/platform/test_orchestration.py (ruff, ruff-format, whitespace) |
✅ passed |
test_output_connector_orchestration |
⚠️ not run. It needs pipeline-manager plus a multihost cluster, which this environment doesn't have |
| feldera-pr-checks gates (tests, flakiness, docs, deps, unsafe, CI) |
✅ none triggered. This is a test-only change, and its waits are bounded polls through wait_for_condition with no bare sleeps |
- Pitfall (medium): this assertion depends on the coordinator change ([adapters] Report a host's committed input records in its step status #7607 and its companion deploy). Before that deploy, any multihost run fails here after the 30 s timeout. So keep it in draft as the description says, and confirm that the multihost Python job runs against the new coordinator image. Don't count a green single-host CI run as validation.
Otherwise LGTM once the 2-host run described in the PR has passed.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Review of #7608 (test-only revert: the paused-output-connector record-counter check runs on every host count again)
What I ran
| Check |
Result |
ruff check / ruff format --check on test_orchestration.py |
pass |
test_output_connector_orchestration |
not run: it needs a pipeline-manager and a multihost coordinator, which this sandbox doesn't have |
| #7607 status |
merged 2026-10-29 |
pitfall (medium): merge order. The PR is no longer marked draft, but the description still says "Draft: merge only after … deployed where the multihost Python tests run". The single-host path is unchanged. On multihost, python-multihost will hit the 30 s timeout on paused connector reports input processed after the pause until the coordinator side is rolled out to that environment. Please confirm the rollout, or re-mark the PR as draft, before it goes into the merge queue. Otherwise multihost CI turns red for a reason the test can't explain.
Gates: this is test-only, so there is no behaviour change, no docs, and no deps to check. The commit message is good, and it explains the why. Flakiness looks fine: the input is an unbounded 100 rec/s datagen, so the condition is guaranteed to become true without a sleep, and the 30 s timeout leaves plenty of headroom.
LGTM once the deployment precondition is met.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|