Redesign `RunnableService` and `SharedData`
- Dominant language
- Rust
- Stars
- 56.8k
- Forks
- 2.9k
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 5
Description
This is addressing two issues:
1. Something that implements `RunnableService` should only be constrained by things related to running the service. "Sharing data" is a separate concern
2. `SharedData` is inherently a violation of [SRP](https://en.wikipedia.org/wiki/Single-responsibility_principle) in that we don't need to put all the shared data in one interface. Under the hood, it might be necessary to have disparate objects held together, but we should provide an interface for those things; i.e. instead of `service.shared_data().foo_bar` we should prefer `service.get_foo_bar()`.
We've had many conversations about this and hopefully this can be a single source of truth to track our decision.
It seems that most people agree that `RunnableService` should no longer have a `SharedData` field. How we want to solve the issue of providing data from inside a service to the outside world is up for debate still.
In the end, the trait should look like this:
```rs
#[async_trait::async_trait]
pub trait RunnableService: Send {
const NAME: &'static str;
type Task: RunnableTask;
type TaskParams: Send;
async fn run_service(
self,
state_watcher: &StateWatcher,
params: Self::TaskParams,
) -> anyhow::Result;
}
```
With `SharedData` and `shared_data()` removed, and `into_task` renamed to `run_service`.
## `SharedData` redesigns
### Green's idea for separate trait
```rs
pub trait ServiceWithSharedData: RunnableService {
type SharedData: Clone + Send + Sync;
fn shared_data(&self) -> Self::SharedData;
}
```
(preliminary trait name)
Where you can optionally describe `SharedData` for some services, but not all of them. This is a good separation of concerns.
Issues:
This solves for #1 but not for #2 of the problems listed above.
### Mitch's idea
TBD WIP TODO LOL
Contributor guide
Research direction
Start by locating the RunnableService and SharedData definitions and reviewing their current uses. Compare those uses with the proposed run_service trait and the separate ServiceWithSharedData idea. Done means the service trait and shared-data access design are agreed, documented in the issue, and implemented consistently.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100