Revamp BackgroundService and rename to Service
#27
Conversation
This file contains 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
github-actions
bot
added
part:tests
llucax
added
type:enhancement
This is not necessary, as properties should be documented as attributes. Signed-off-by: Leandro Lucarella <[email protected]>
The name `BackgroundService` is a bit too verbose now in the context of an asyncio library. Services are usually already understood as running in the background, so the `Background` prefix is redundant and makes the class name too long. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
This will be used in the future to allow overriding where to create tasks (for example the default `asyncio` loop, a custom `loop` or even a `TaskGroup`). Signed-off-by: Leandro Lucarella <[email protected]>
This change adds a new parameter to the Service class, `task_creator`, that will be used to create tasks. This is useful to allow the user to use different task creators, like the `asyncio` module or a `TaskGroup`. A method to create tasks was also added to the Service class, that uses the task creator object to create the tasks, hopefully making it easier for users not to forget to register tasks managed by the service in the `self._tasks` set. Signed-off-by: Leandro Lucarella <[email protected]>
It is common that users forget to handle exceptions properly in their tasks, and since services tasks are supposed to run forever, there are no many opportunities to await for their completion appropriately and handle the failure. With this at least we make sure we will get a log message when a task raises an exception, so we can at least know what happened and they just silently disappear. Signed-off-by: Leandro Lucarella <[email protected]>
This makes it much easier when debugging to know which service spawned each task. Signed-off-by: Leandro Lucarella <[email protected]>
The warning is not very clear and might suggest users need to hold references to the service's tasks, which is not correct, only a reference to the service itself should be saved. Signed-off-by: Leandro Lucarella <[email protected]>
By having both mixed up, it was hard to separate documentation for users of services, and developers wanting to implement services. It also gave access to service users to internals for service implementation, like `create_task`, which is not ideal. Now users can use the `Service` type to refer to services generically and implementors can use `ServiceBase` to have a sane starting point to implement a service. Signed-off-by: Leandro Lucarella <[email protected]>
We don't really need to make it a public method, as one can just use `await service` to await for it. It is still good to have it separate as implementing `__await__` is easier based on another awaitable. Signed-off-by: Leandro Lucarella <[email protected]>
Marenz
approved these changes
Jul 29, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 9, 2024
`asyncio.TaskGroup` is a very convenient construct when using parallelization for doing calculations for example, where the results for all the tasks need to be merged together to produce a final result. In this case if one of the tasks fails, it makes sense to cancel the others and abort as soon as possible, as any further calculations would be thrown away. This PR introduces a new `PersistentTaskGroup` class, intended to help managing a group of tasks that should persist even if other tasks in the group fail, usually by either only discarding the failed task or by restarting it somehow. The `ServiceBase` class is updated to use a `PersistentTaskGroup` underneath, and it is simplified for single-task services by making the service driven by a single `main` task, which can use the new group to monitor sub-tasks and act accordingly. This is part of #27 and #9.
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.
The name
BackgroundServiceis a bit too verbose now in the context of an asyncio library. Services are usually already understood as running in the background, so theBackgroundprefix is redundant and makes the class name too long.The
Serviceclass is made an abstract base class intended for end users that just want to use services, and a newServiceBaseclass is added for developers that want to implement services. This way, the documentation can be separated between the two use cases and theServiceclass can be used as a generic type for services. We also avoid leaking implementation details to services users.The
ServiceBaseclass also have a new methodcreate_taskthat is used to create tasks in the service, and a newTaskCreatorprotocol is added to allow overriding how tasks are created (for example, using a customloopor aTaskGroup).create_taskautomatically adds the task to the internal set, so hopefully users won't forget so easily to register tasks.The
create_taskmethod also have a newlog_exceptionsparameter that allows to log exceptions raised by tasks. This is useful because services tasks are supposed to run forever, and there are no many opportunities to await for their completion appropriately and handle the failure. With this at least we make sure we will get a log message when a task raises an exception, so we can at least know what happened and they just silently disappear.Fixes #8, improves #9.