-
Notifications
You must be signed in to change notification settings - Fork 210
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
Refactor server #5264
Refactor server #5264
Conversation
714f596
to
2fcdcf0
Compare
14b62a1
to
a6b4e62
Compare
5ead912
to
dcf0183
Compare
dcf0183
to
7c3c8f5
Compare
pub struct ConsensusEngine { | ||
pub modules: ServerModuleRegistry, | ||
pub db: Database, | ||
pub keychain: Keychain, | ||
pub federation_api: DynGlobalApi, | ||
pub cfg: ServerConfig, | ||
pub submission_receiver: Receiver<ConsensusItem>, | ||
pub shutdown_receiver: watch::Receiver<Option<u64>>, | ||
pub last_ci_by_peer: Arc<RwLock<BTreeMap<PeerId, u64>>>, | ||
/// Just a string version of `cfg.local.identity` for performance | ||
self_id_str: String, | ||
pub self_id_str: String, | ||
/// Just a string version of peer ids for performance | ||
peer_id_str: Vec<String>, | ||
connection_status_channels: Arc<RwLock<BTreeMap<PeerId, PeerConnectionStatus>>>, | ||
task_group: TaskGroup, | ||
pub peer_id_str: Vec<String>, | ||
pub connection_status_channels: Arc<RwLock<BTreeMap<PeerId, PeerConnectionStatus>>>, | ||
pub task_group: TaskGroup, | ||
} |
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.
Making stuff pub is ok for now, but this is an internal-only struct and many of its fields are related leading to invariants that need to be upheld, so making everything pub could be kinda misleading (pub fields can be written to arbitrarily in a good API imo, if not there should be a setter) and we probably want to tune it down a bit.
review club: @joschisan what ended up being the solution for the problem you saw in CI? Also, it would help in the review club if there was a description. |
review club: dpc relayed that the problem was resolved by removing the diff that caused it. We should followup with an investigation to see what we were getting the Edit: more context in discord |
Just moving code around.