Add method to restart stream when retry strategy is exhausted

[Marenz]

No description provided.

[llucax]

Do we really need this? Why not just setting the retry limit to None (which is the default BTW)?

• https://frequenz-floss.github.io/frequenz-client-base-python/latest/reference/frequenz/client/base/streaming/#frequenz.client.base.streaming.GrpcStreamBroadcaster.__init__
• https://frequenz-floss.github.io/frequenz-client-base-python/latest/reference/frequenz/client/base/retry/#frequenz.client.base.retry.LinearBackoff.__init__

[Marenz]

Do we really need this? Why not just setting the retry limit to None (which is the default BTW)?

I am setting it to None. But how I am ever gonna restart that stream now?

[llucax]

I am setting it to None. But how I am ever gonna restart that stream now?

Why do you want to restart it if it restarts automatically continuously. Maybe you can describe the use case you have in mind more in detail?

No it's not?!

Yes, indefinitely.

frequenz-client-base-python/src/frequenz/client/base/streaming.py

Lines 50 to 52 in dfdfaf3

	self._retry_strategy = (
	retry.LinearBackoff() if retry_strategy is None else retry_strategy.copy()
	)

frequenz-client-base-python/src/frequenz/client/base/retry.py

Lines 77 to 84 in dfdfaf3

	class LinearBackoff(Strategy):
	"""Provides methods for calculating the interval between retries."""
	def __init__(
	self,
	interval: float = DEFAULT_RETRY_INTERVAL,
	jitter: float = DEFAULT_RETRY_JITTER,
	limit: int | None = None,

[Marenz]

Well, I want an exception to be thrown when the connection fails.

Then I handle it etc.. and then I want to try it again. But i can't, when the retry strategy is exhausted, the whole streamer instance is from that point on useless.

[llucax]

Then the documentation is wrong :P

No, why? It says it retries indefinitely.

Well, I want an exception to be thrown when the connection fails.

So you don't want to retry indefinitely, you want to have more control, is that the use case?

[Marenz]

Yes, that's the usecase. Remember, we wanted to put the handling a level higher?

[llucax]

OK, I guess I was confused by the PR title, because in this case you don't want a retry strategy at all, right?

[Marenz]

Yes. but I can have that already when I just say "limit=0".
The title just refers to the condition for when it's needed: When the retry condition is exhausted and the channel is closed.

[llucax]

OK, looking at it again with the new information, what would be the advantage of being able to restart it compared to create a new streamer? You are rebuilding the whole internal state (recreating the channel, tasks, etc.), so from the efficiency point of view I guess it doesn't make a lot of difference.

For your use case it would be more inconvenient to re-create the streamer instead? In any this PR is fixing a sort of bug, it is not great that if you call start() after it was stopped it will misbehave.

But I was wondering if we could still keep the channel opened (or even the task alive). As for the channel, right now it is closed to stop the receivers, I think, so I wonder if this is something we want to do or not.

[llucax]

Oh, I guess in your use case you'll just create a new receiver for the streamer when the current receiver stops? If so I can see how it is simpler to use than recreating the whole streamer.

[llucax]

I would update the PR description and commit message to something like "Allow (re)starting a stopped streamer" to make it less confusing, other than that I'm fine with merging this as is and leave other ideas for future improvements.

[Marenz]

I mean, i can also recreate the whole instance, sure frequenz-floss/frequenz-client-dispatch-python#94

[llucax]

I'm also fine with both, I wanted to understand the use case better. Both PRs are approved, I leave it to you to chose what to use.