Skip to content

Conversation

@llucax
Copy link
Contributor

@llucax llucax commented Oct 27, 2025

The timeout counts for the whole duration of the call, not each individual streamed message, so it always times out, as we expect this stream to basically last forever.

On top of this, disconnecting the stream seems to trigger some bug in the SDK, so we better avoid it for now.

Copilot AI review requested due to automatic review settings October 27, 2025 14:24
@llucax llucax requested review from a team as code owners October 27, 2025 14:24
@llucax llucax self-assigned this Oct 27, 2025
@llucax llucax added this to the v0.18.0 milestone Oct 27, 2025
@github-actions github-actions bot added the part:client Affects the client code label Oct 27, 2025
@llucax llucax enabled auto-merge October 27, 2025 14:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the timeout parameter from a gRPC streaming call to prevent premature disconnections. The timeout was incorrectly applied to the entire stream duration rather than individual messages, causing issues with long-lived streaming connections that are expected to run indefinitely.

Key Changes:

  • Removed the timeout=DEFAULT_GRPC_CALL_TIMEOUT parameter from the receive_component_data_samples_stream method's gRPC call

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@llucax llucax added the type:bug Something isn't working label Oct 27, 2025
@llucax llucax requested a review from shsms October 27, 2025 14:24
@llucax llucax added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Oct 27, 2025
The timeout counts for the whole duration of the call, not each
individual streamed message, so it always times out, as we expect this
stream to basically last forever.

On top of this, disconnecting the stream seems to trigger some bug in
the SDK, so we better avoid it for now.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax force-pushed the rm-stream-timeout branch from 2eff085 to 1fa9aff Compare October 27, 2025 14:37
@github-actions github-actions bot added the part:tests Affects the unit, integration and performance (benchmarks) tests label Oct 27, 2025
@llucax llucax merged commit b476f28 into frequenz-floss:v0.18.x Oct 27, 2025
5 checks passed
@llucax llucax deleted the rm-stream-timeout branch October 28, 2025 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmd:skip-release-notes It is not necessary to update release notes for this PR part:client Affects the client code part:tests Affects the unit, integration and performance (benchmarks) tests type:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants