Close receiver when stopping LatestValueCache#495
Close receiver when stopping LatestValueCache#495shsms wants to merge 4 commits intofrequenz-floss:v1.x.xfrom
LatestValueCache#495Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates LatestValueCache to take ownership of its Receiver and ensure the receiver is closed when the cache is stopped, preventing unused receivers from remaining attached to channels.
Changes:
- Document that
LatestValueCachetakes ownership of the receiver and will close it on stop. - Call
self._receiver.close()fromLatestValueCache.stop().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -109,6 +112,7 @@ async def _run(self) -> None: | |||
|
|
|||
| async def stop(self) -> None: | |||
| """Stop the cache.""" | |||
There was a problem hiding this comment.
stop() now closes the underlying receiver, but stop()'s docstring still only says "Stop the cache". Please update the method docstring to mention that it also closes the owned receiver, so callers don't accidentally expect to keep using it after stopping.
| """Stop the cache.""" | |
| """Stop the cache and close the owned receiver. | |
| After this method is called, the underlying receiver is closed and | |
| must not be used anymore. | |
| """ |
|
|
||
| Takes ownership of the receiver. When the cache is stopped, the receiver | ||
| will be closed. |
There was a problem hiding this comment.
Since this is a behavior/ownership change, consider also updating the module/package-level documentation for LatestValueCache (e.g., the module docstring and/or the entry in frequenz.channels.__init__ docs) so users reading the top-level docs learn that stop() closes the receiver.
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
0acba17 to
67d58a8
Compare
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
2d75df7 to
a812c29
Compare
No description provided.