You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While working on #1066 and the integration tests for the roles, I noticed a few technical debts that would need to be addressed in a future refactor:
** Note that the goal of this issue is to document the different problems within the roles code, but we are not planning to resolve them until we are done with #845.
The following points are related almost always to all of the roles:
Problem: async-channel is used within tokio context which can result in an unexpected bugs
Solution: use tokio channels https://tokio.rs/tokio/tutorial/channels
Problem: Some struct(s) function(s) have weird signatures like self_: &Arc<Mutex<Self>>
Solution: Wrap only the properties that need to be mutated in a Mutex
Problem: Duplicate code between the different roles. Some roles need Downstream service, others need Upstream. Usually this is just a server.
Solution: use a common definition for Upstream, Downstream across the roles where possible
Problem: Config properties are not clear and not grouped in structs
Solution: Group properties into structs with more clear names
Problem: Roles have more responsibilities than needed. For example the pool role is required to connect to the TemplateProvider instead of receiving the TemplateProvider connection as an input.
Solution: Minimize code inside role::start functions across the different roles to only do whats that role is responsible for.
The text was updated successfully, but these errors were encountered:
we should remove async-channel cause most of the time is not needed or we can slightly change the implementation, that would result in a dependency removed that is good. Said that I want to add that I'm pretty sure that using them withing tokio context is ok.
This make sense, but sometimes is faster to have one single Mutex than a lot of small ones, so we should go case by case here.
We use the internal Mutex cause it have safe_lock that IMO is an interface fair superior than lock, it make a lot of foot guns harder or not possible (lock between await, not releasing the lock ecc ecc). Also keep in mind that tokio Mutex is not the right choice, most of the time is better to use std Mutex, you can find more about it on the tokio documentation.
While working on #1066 and the integration tests for the
roles
, I noticed a few technical debts that would need to be addressed in a future refactor:** Note that the goal of this issue is to document the different problems within the roles code, but we are not planning to resolve them until we are done with #845.
The following points are related almost always to all of the roles:
Problem:
async-channel
is used withintokio
context which can result in an unexpected bugsSolution: use
tokio
channels https://tokio.rs/tokio/tutorial/channelsProblem: Some struct(s) function(s) have weird signatures like
self_: &Arc<Mutex<Self>>
Solution: Wrap only the properties that need to be mutated in a
Mutex
Problem: An internal
Mutex
implementation is usedSolution: use https://docs.rs/tokio/latest/tokio/sync/struct.Mutex.html instead
Problem: Duplicate code between the different roles. Some roles need
Downstream
service, others needUpstream
. Usually this is just a server.Solution: use a common definition for
Upstream
,Downstream
across theroles
where possibleProblem: Config properties are not clear and not grouped in structs
Solution: Group properties into structs with more clear names
Problem: Roles have more responsibilities than needed. For example the
pool
role is required to connect to theTemplateProvider
instead of receiving theTemplateProvider
connection as an input.Solution: Minimize code inside
role::start
functions across the different roles to only do whats that role is responsible for.The text was updated successfully, but these errors were encountered: