-
Notifications
You must be signed in to change notification settings - Fork 0
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
20 datafusion of all three topics into the report #24
base: main
Are you sure you want to change the base?
20 datafusion of all three topics into the report #24
Conversation
merger/src/consumer.rs
Outdated
} | ||
|
||
fn detect_event_type_topic2(payload: &Value) -> EventType { | ||
if payload.get("truck_id").is_some() && payload.get("latitude").is_some() { |
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.
Instead of checking for the existence of two attributes, could you try to instantiating a Position
object and in case of an error you return a EventType::Unknown
.
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.
Or can you just get the "type" field and return the correct type ? E.g. :
match payload.get("type") {
Some(type) => match ...
None => EventType::Unknown
}
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.
The issue is, we do not have a field that is present in all of the 4 eventType. "type" is present for driver and truck, but not for the other two.
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.
The field "type_" : "position" will be added to the Position element, to be able to use this method.
The types can be as followed : driver, truck, start, end, rest and position
true | ||
} else { | ||
error!("Kafka is unreachable !"); | ||
error!("Kafka is unreachable!"); |
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.
Please use the tracing crate instead of the standard one. Tracing provides great features such as log formatting (for json) or a loki exporter which are mandatory in a cloud environment
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.
Done
merger/src/consumer.rs
Outdated
// List of topics to subscribe to | ||
let topics = vec!["topic1", "topic2", "topic3"]; | ||
// Shared HashMap for storing parsed messages | ||
let shared_store: Arc<Mutex<HashMap<String, Vec<Value>>>> = Arc::new(Mutex::new(HashMap::new())); |
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.
Wouldn't it be better to reconstruct the objects and store the messages in different hashmaps ?
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.
Yes this is the end goal, I am not here yet
merger/src/consumer.rs
Outdated
let shared_store = Arc::clone(&shared_store); | ||
let topic = topic.clone(); | ||
|
||
thread::spawn(move || { |
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.
Use tokio spawn instead of thread spawn. Actually tokio provides you a managed thread pool so you don't have to manage it yourself. Since you will be doing a lot of IO bound operations you should reallly use tokio since it was made for that
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.
Done
consumer | ||
.subscribe(&[&topic]) | ||
.expect("Subscription to topic failed"); | ||
|
||
loop { |
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.
If we want to do it the clean way, you should implement a graceful shutdown that listen for the ctrl+c unix event (interrupt). You can do that with a channel like in go https://doc.rust-lang.org/rust-by-example/std_misc/channels.html
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.
outdated
No description provided.