Skip to content

Refactor: config actor#111

Closed
nahharris wants to merge 10 commits intokworkflow:unstablefrom
nahharris:refactor/tokio-config
Closed

Refactor: config actor#111
nahharris wants to merge 10 commits intokworkflow:unstablefrom
nahharris:refactor/tokio-config

Conversation

@nahharris
Copy link
Contributor

@nahharris nahharris commented Mar 4, 2025

In this PR i'm drafting my proposal to use the actor model with the Config. Several minor refactors where done (aka async fn/await) all over the code. But the core of this is the design chosen for the config actor.

The design

We still have a Config struct and most of the methods implemented before. But the getters and setters where changed. The actor model consists of messages being passed around, since most config options are strings, i thought it would be useful to use a single message to handle both getting/setting a string, just created a enum StringOpt to identify which option we wanna handle. Same for usize config options. For other options I created messages for each of them.

The ConfigActor trait behaves similar to the LoggerActor trait, by abstracting the call to send. But this time most messages are async, since most of them return a response. And this reminds me of clarifying the response model adopted.

Response Model

Again, the actor model is all about sending and receiving responses. So far we defined how to send messages, by using a tokio::sync::mspc::Sender<Command> and catching it in a dedicated task. So for messages that need a response, we will have a tokio::sync::oneshot::Sender<T> in the payload of the message, this way the receiver has a channel to send a response back with any value we need. The only drawback is that we cannot use &. So to return a string from out configuration, we need to clone it. But since our strings are not that long, it won't be a huge performance hit. It will be worse, of course, but not the

This approach is the one i'm sticking to during the continuation of this project and hope other developers keep this pattern

TODO

  • remove dead code
  • testing
  • serde message rather than cloning the whole thing
  • env actor

@nahharris nahharris force-pushed the refactor/tokio-config branch 4 times, most recently from 52afc35 to 828bfbd Compare March 7, 2025 13:52
Add `tokio` crate as a dependency.

Signed-off-by: OJarrisonn <j.h.m.t.v.10@gmail.com>
Here i've created the implementation of the logger actor. This actor is
responsible for logging messages.

The `Logger` struct is a simple refactor of the old
`app::logging::Logger` it is responsible for storing messages that will
be later printed to the stdout and also to provide methods such as `spawn`

`spawn` is responsible for creating a dedicated task that will handle
all the messages received from all the transmitters.

The `LoggerTx` is a transmitter that is responsible by sending messages
down to the running actor. This is what should be used accross the code
base to communicate with the actor.

The `MockLoggerTx` is a mock version of this logger which does nothing
at all.

Finally but not least, the `LoggerActor` trait which unifies the
interface of `LoggerTx` and `MockLoggerTx`. This permits the swaping of
the loggers for testing scenarios.

Signed-off-by: OJarrisonn <j.h.m.t.v.10@gmail.com>
Removes the old logger implementation for cleanup purposes

Signed-off-by: OJarrisonn <j.h.m.t.v.10@gmail.com>
Adapts the `utils` testing module to support the new async setup
function.

Signed-off-by: OJarrisonn <j.h.m.t.v.10@gmail.com>
Adds a simple description to the `logger` field in the `App` struct

Signed-off-by: OJarrisonn <j.h.m.t.v.10@gmail.com>
@nahharris nahharris force-pushed the refactor/tokio-config branch from 828bfbd to 45cfd35 Compare March 15, 2025 17:48
This actor is a way to improve patch-hub testing suit. It's just a
wrapper to `std::env` to let us manipulate environment variables. But it
also has a mock variant which won't touch the real environment variables
but use a simple `HashMap` instead

With this, the later config tests can be run in parallel

Signed-off-by: OJarrisonn <j.h.m.t.v.10@gmail.com>
The config actor will be responsible for managing config options in
patch-hub. It's a simple store with capability to handle data
serialization and deserialization.

Signed-off-by: OJarrisonn <j.h.m.t.v.10@gmail.com>
Adapts the code to use the new config actor instead of the old config
implementation. This mean that several functions became async.

There is a little trick that will be solve later since we cannot use
async functions to draw on the terminal so we pass the `page_size` value
directly instead of gathering it from the `app.config` in the `draw_ui`
function.

Signed-off-by: OJarrisonn <j.h.m.t.v.10@gmail.com>
Deletes the old config files and move/adapt the config files to the new
config

Signed-off-by: OJarrisonn <j.h.m.t.v.10@gmail.com>
@nahharris nahharris force-pushed the refactor/tokio-config branch from 45cfd35 to a33f515 Compare March 15, 2025 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants