-
Notifications
You must be signed in to change notification settings - Fork 99
Using Bilrost for GetNodeStatus messages #3172
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
Conversation
#[prost(target = "crate::protobuf::cluster::RunMode")] | ||
#[strum(serialize_all = "snake_case")] | ||
pub enum RunMode { | ||
Leader, | ||
Follower, | ||
Follower = 0, |
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.
Bilrost requires numeric enums to have an "EmptyState" which is always the variant with zero value. It's up to us to introduce Unknown
variant (like what we do with protobuf) or assign the 0 value to one of the existing variants.
5172955
to
2fa50cc
Compare
1c9a326
to
08b4d59
Compare
0f73075
to
b1cbba0
Compare
5ed1d99
to
d132aae
Compare
@@ -12,6 +12,7 @@ use std::collections::BTreeMap; | |||
use std::time::{Duration, Instant}; | |||
|
|||
use prost_dto::IntoProst; | |||
use restate_encoding::NetSerde; |
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.
it'd be ever-so-slightly nicer if this follows our import ordering convention.
Also introduce two new derive macros: - `BilrostNewType` to use with all ID types or any tuple with a single item. This will effectively flatten this tuple to its inner type. Given that the inner field is bilrost serializable - `NetworkMessage` which must be derived for each message that is going to be send over the network
Using Bilrost for GetNodeStatus messages
Stack created with Sapling. Best reviewed with ReviewStack.