Revert retry strategy changes for streaming#204
Merged
llucax merged 7 commits intofrequenz-floss:v0.x.xfrom Dec 18, 2025
Merged
Revert retry strategy changes for streaming#204llucax merged 7 commits intofrequenz-floss:v0.x.xfrom
llucax merged 7 commits intofrequenz-floss:v0.x.xfrom
Conversation
This reverts commit 1159958. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This reverts commit 99b6853. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This reverts commit eef9557. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This reverts commit 30ee943. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This reverts commit a345940. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
…event" This reverts commit 043e121. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This reverts commit 23137ba. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
There was a problem hiding this comment.
Pull request overview
This PR reverts streaming retry strategy changes from PR #148 due to issues with initial_metadata() hanging indefinitely. The changes simplify the streaming implementation by removing the initial metadata check and associated retry reset logic.
Key Changes:
- Removed
initial_metadata()call that was used to detect successful connections - Removed retry strategy reset mechanism that triggered after first message received
- Simplified test mocks to use plain async iterables instead of gRPC call objects
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/frequenz/client/base/streaming.py | Reverted stream_method signature from gRPC UnaryStreamCall to AsyncIterable, removed initial_metadata check and retry reset logic |
| tests/streaming/test_grpc_stream_broadcaster.py | Simplified test mocks to use plain async iterators, removed initial_metadata mocking infrastructure and retry reset test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Reverts code only present in a yanked version, so no need to update the release notes. |
shsms
approved these changes
Dec 18, 2025
Merged
via the queue into
frequenz-floss:v0.x.x
with commit Dec 18, 2025
4d08711
11 of 13 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR reverts most of #148 because
initial_metadata()doesn't work as expected and hangs forever.We need to investigate this further and find another mechanism to be able to detect when a connection is successful or other indication to reset the retry strategy properly send more reliable start events.