-
Notifications
You must be signed in to change notification settings - Fork 6
Some final preparations #108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
01f9070
Add method to check if a type is currently being dispatched
Marenz eefdd7f
Make actor_factory function async for more flexibility
Marenz 3ccd50c
`with` now waits till dispatch is initialized
Marenz 90ee0a2
Raise when trying to create a receiver while disconnected
Marenz ba246c4
Only send currently active dispatches to new receivers.
Marenz 5b84b4d
Remove debug log from tests
Marenz 42e7815
Fix tests
Marenz 596ee48
Update and correct documentation, release-notes and README
Marenz 9faa88b
Dispatches that failed to start will now be retried after a delay.
Marenz d06291e
Delete actors after stopping them
Marenz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,9 +7,10 @@ | |||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||||
| from collections.abc import Callable | ||||||||||||||||||||||||||||||||||
| from dataclasses import dataclass | ||||||||||||||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||||||||||||||
| from datetime import timedelta | ||||||||||||||||||||||||||||||||||
| from typing import Any, Awaitable | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from frequenz.channels import Broadcast, Receiver | ||||||||||||||||||||||||||||||||||
| from frequenz.channels import Broadcast, Receiver, select | ||||||||||||||||||||||||||||||||||
| from frequenz.client.dispatch.types import TargetComponents | ||||||||||||||||||||||||||||||||||
| from frequenz.sdk.actor import Actor, BackgroundService | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
@@ -116,29 +117,77 @@ async def main(): | |||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| microgrid_id = 1 | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| dispatcher = Dispatcher( | ||||||||||||||||||||||||||||||||||
| async with Dispatcher( | ||||||||||||||||||||||||||||||||||
| microgrid_id=microgrid_id, | ||||||||||||||||||||||||||||||||||
| server_url=url, | ||||||||||||||||||||||||||||||||||
| key=key | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| dispatcher.start() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| status_receiver = dispatcher.new_running_state_event_receiver("EXAMPLE_TYPE") | ||||||||||||||||||||||||||||||||||
| ) as dispatcher: | ||||||||||||||||||||||||||||||||||
| status_receiver = dispatcher.new_running_state_event_receiver("EXAMPLE_TYPE") | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| managing_actor = ActorDispatcher( | ||||||||||||||||||||||||||||||||||
| actor_factory=MyActor.new_with_dispatch, | ||||||||||||||||||||||||||||||||||
| running_status_receiver=status_receiver, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| managing_actor = ActorDispatcher( | ||||||||||||||||||||||||||||||||||
| actor_factory=MyActor.new_with_dispatch, | ||||||||||||||||||||||||||||||||||
| running_status_receiver=status_receiver, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| await run(managing_actor) | ||||||||||||||||||||||||||||||||||
| await run(managing_actor) | ||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def __init__( | ||||||||||||||||||||||||||||||||||
| class RetryFailedDispatches: | ||||||||||||||||||||||||||||||||||
| """Manages the retry of failed dispatches.""" | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def __init__(self, retry_interval: timedelta) -> None: | ||||||||||||||||||||||||||||||||||
| """Initialize the retry manager. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||||||||
| retry_interval: The interval between retries. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| self._retry_interval = retry_interval | ||||||||||||||||||||||||||||||||||
| self._channel = Broadcast[Dispatch](name="retry_channel") | ||||||||||||||||||||||||||||||||||
| self._sender = self._channel.new_sender() | ||||||||||||||||||||||||||||||||||
| self._tasks: set[asyncio.Task[None]] = set() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def new_receiver(self) -> Receiver[Dispatch]: | ||||||||||||||||||||||||||||||||||
| """Create a new receiver for dispatches to retry. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||||||
| The receiver. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| return self._channel.new_receiver() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def retry(self, dispatch: Dispatch) -> None: | ||||||||||||||||||||||||||||||||||
| """Retry a dispatch. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||||||||
| dispatch: The dispatch information to retry. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| task = asyncio.create_task(self._retry_after_delay(dispatch)) | ||||||||||||||||||||||||||||||||||
| self._tasks.add(task) | ||||||||||||||||||||||||||||||||||
| task.add_done_callback(self._tasks.remove) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| async def _retry_after_delay(self, dispatch: Dispatch) -> None: | ||||||||||||||||||||||||||||||||||
| """Retry a dispatch after a delay. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||||||||
| dispatch: The dispatch information to retry. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| _logger.info( | ||||||||||||||||||||||||||||||||||
| "Will retry dispatch %s after %s", | ||||||||||||||||||||||||||||||||||
| dispatch.id, | ||||||||||||||||||||||||||||||||||
| self._retry_interval, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| await asyncio.sleep(self._retry_interval.total_seconds()) | ||||||||||||||||||||||||||||||||||
| _logger.info("Retrying dispatch %s now", dispatch.id) | ||||||||||||||||||||||||||||||||||
| await self._sender.send(dispatch) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def __init__( # pylint: disable=too-many-arguments, too-many-positional-arguments | ||||||||||||||||||||||||||||||||||
| self, | ||||||||||||||||||||||||||||||||||
| actor_factory: Callable[[DispatchInfo, Receiver[DispatchInfo]], Actor], | ||||||||||||||||||||||||||||||||||
| actor_factory: Callable[ | ||||||||||||||||||||||||||||||||||
| [DispatchInfo, Receiver[DispatchInfo]], Awaitable[Actor] | ||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||
| running_status_receiver: Receiver[Dispatch], | ||||||||||||||||||||||||||||||||||
| dispatch_identity: Callable[[Dispatch], int] | None = None, | ||||||||||||||||||||||||||||||||||
| retry_interval: timedelta | None = timedelta(seconds=60), | ||||||||||||||||||||||||||||||||||
| ) -> None: | ||||||||||||||||||||||||||||||||||
| """Initialize the dispatch handler. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
@@ -148,6 +197,7 @@ def __init__( | |||||||||||||||||||||||||||||||||
| running_status_receiver: The receiver for dispatch running status changes. | ||||||||||||||||||||||||||||||||||
| dispatch_identity: A function to identify to which actor a dispatch refers. | ||||||||||||||||||||||||||||||||||
| By default, it uses the dispatch ID. | ||||||||||||||||||||||||||||||||||
| retry_interval: The interval between retries. If `None`, retries are disabled. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| super().__init__() | ||||||||||||||||||||||||||||||||||
| self._dispatch_identity: Callable[[Dispatch], int] = ( | ||||||||||||||||||||||||||||||||||
|
|
@@ -161,6 +211,11 @@ def __init__( | |||||||||||||||||||||||||||||||||
| name="dispatch_updates_channel", resend_latest=True | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| self._updates_sender = self._updates_channel.new_sender() | ||||||||||||||||||||||||||||||||||
| self._retrier = ( | ||||||||||||||||||||||||||||||||||
| ActorDispatcher.RetryFailedDispatches(retry_interval) | ||||||||||||||||||||||||||||||||||
| if retry_interval | ||||||||||||||||||||||||||||||||||
| else None | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def start(self) -> None: | ||||||||||||||||||||||||||||||||||
| """Start the background service.""" | ||||||||||||||||||||||||||||||||||
|
|
@@ -174,7 +229,8 @@ async def _start_actor(self, dispatch: Dispatch) -> None: | |||||||||||||||||||||||||||||||||
| options=dispatch.payload, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| actor: Actor | None = self._actors.get(self._dispatch_identity(dispatch)) | ||||||||||||||||||||||||||||||||||
| identity = self._dispatch_identity(dispatch) | ||||||||||||||||||||||||||||||||||
| actor: Actor | None = self._actors.get(identity) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if actor: | ||||||||||||||||||||||||||||||||||
| sent_str = "" | ||||||||||||||||||||||||||||||||||
|
|
@@ -189,21 +245,28 @@ async def _start_actor(self, dispatch: Dispatch) -> None: | |||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||
| _logger.info("Starting actor for dispatch type %r", dispatch.type) | ||||||||||||||||||||||||||||||||||
| actor = self._actor_factory( | ||||||||||||||||||||||||||||||||||
| actor = await self._actor_factory( | ||||||||||||||||||||||||||||||||||
| dispatch_update, | ||||||||||||||||||||||||||||||||||
| self._updates_channel.new_receiver(limit=1, warn_on_overflow=False), | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| self._actors[self._dispatch_identity(dispatch)] = actor | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| actor.start() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| except Exception as e: # pylint: disable=broad-except | ||||||||||||||||||||||||||||||||||
| _logger.error( | ||||||||||||||||||||||||||||||||||
| "Failed to start actor for dispatch type %r: %s", | ||||||||||||||||||||||||||||||||||
| "Failed to start actor for dispatch type %r", | ||||||||||||||||||||||||||||||||||
| dispatch.type, | ||||||||||||||||||||||||||||||||||
| e, | ||||||||||||||||||||||||||||||||||
| exc_info=True, | ||||||||||||||||||||||||||||||||||
| exc_info=e, | ||||||||||||||||||||||||||||||||||
|
Comment on lines
256
to
+259
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can just use |
||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| if self._retrier: | ||||||||||||||||||||||||||||||||||
| self._retrier.retry(dispatch) | ||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||
| _logger.error( | ||||||||||||||||||||||||||||||||||
| "No retry mechanism enabled, dispatch %r failed", dispatch | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||
| # No exception occurred, so we can add the actor to the list | ||||||||||||||||||||||||||||||||||
| self._actors[identity] = actor | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| async def _stop_actor(self, stopping_dispatch: Dispatch, msg: str) -> None: | ||||||||||||||||||||||||||||||||||
| """Stop all actors. | ||||||||||||||||||||||||||||||||||
|
|
@@ -212,17 +275,33 @@ async def _stop_actor(self, stopping_dispatch: Dispatch, msg: str) -> None: | |||||||||||||||||||||||||||||||||
| stopping_dispatch: The dispatch that is stopping the actor. | ||||||||||||||||||||||||||||||||||
| msg: The message to be passed to the actors being stopped. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| if actor := self._actors.pop(self._dispatch_identity(stopping_dispatch), None): | ||||||||||||||||||||||||||||||||||
| actor: Actor | None = None | ||||||||||||||||||||||||||||||||||
| identity = self._dispatch_identity(stopping_dispatch) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| actor = self._actors.get(identity) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if actor: | ||||||||||||||||||||||||||||||||||
| await actor.stop(msg) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| del self._actors[identity] | ||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||
| _logger.warning( | ||||||||||||||||||||||||||||||||||
| "Actor for dispatch type %r is not running", stopping_dispatch.type | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+281
to
290
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip:
Suggested change
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| async def _run(self) -> None: | ||||||||||||||||||||||||||||||||||
| """Wait for dispatches and handle them.""" | ||||||||||||||||||||||||||||||||||
| async for dispatch in self._dispatch_rx: | ||||||||||||||||||||||||||||||||||
| await self._handle_dispatch(dispatch=dispatch) | ||||||||||||||||||||||||||||||||||
| """Run the background service.""" | ||||||||||||||||||||||||||||||||||
| if not self._retrier: | ||||||||||||||||||||||||||||||||||
| async for dispatch in self._dispatch_rx: | ||||||||||||||||||||||||||||||||||
| await self._handle_dispatch(dispatch) | ||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||
| retry_recv = self._retrier.new_receiver() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| async for selected in select(retry_recv, self._dispatch_rx): | ||||||||||||||||||||||||||||||||||
| if retry_recv.triggered(selected): | ||||||||||||||||||||||||||||||||||
| self._retrier.retry(selected.message) | ||||||||||||||||||||||||||||||||||
| elif self._dispatch_rx.triggered(selected): | ||||||||||||||||||||||||||||||||||
| await self._handle_dispatch(selected.message) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| async def _handle_dispatch(self, dispatch: Dispatch) -> None: | ||||||||||||||||||||||||||||||||||
| """Handle a dispatch. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now you need to finalize this object too, otherwise when the dispatcher that instantiates it is destroyed, all these tasks will be dangling until the garbage collection destroys them. My feeling is this class needs to end up being a
BackgroundServicetoo.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those tasks only hang around 60 seconds or so anyway then they destroy themselves through the done callback below..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it is not too bad, but there could be situation on program termination that the loop can emit warnings when there are pending tasks and the loop is stopped, specially in tests. So I agree it is not urgent, but eventually I would implement proper finalization for this, as most of the time that we say "meh, who cares about finalization", it eventually comes back to bite us :D