Howl send vc00 - #90
jackbdoughty wants to merge 18 commits into
Conversation
… not send vc00 yet.
|
I think there might be a misunderstanding here - enabled vetoes are the ones that a user has said they care about, probabilities are the ones actually applied to events. So from a Saluki perspective it |
| .expect("Failed to get system time") | ||
| .as_nanos() | ||
| .try_into() | ||
| .expect("This will fail after April 11th, 2262"); |
There was a problem hiding this comment.
ah crap what a showstopper
There was a problem hiding this comment.
We should petition for a national day of mourning
rerpha
left a comment
rerpha
left a comment
There was a problem hiding this comment.
looks good - i think we want to probably also send a veto config update each time a run start is sent too (this is what kafka_dae_control does)
| let mut config: ClientConfig = ClientConfig::new(); | ||
| config.set("bootstrap.servers", conf.broker); | ||
|
|
||
| if let Some(kafka_options) = &conf.kafka_config { | ||
| for option in kafka_options { | ||
| println!( | ||
| "Setting Kafka config option {}={}", | ||
| option.key, option.value | ||
| ); | ||
| config.set(&option.key, &option.value); | ||
| } | ||
| } |
There was a problem hiding this comment.
i think it'd be good to pull this out to a separate function - we will probably use it elsewhere at some point. it'd also probably be easy to unit test!
There was a problem hiding this comment.
(feel free to not do this in this PR btw, can appreciate it may scope creep the ticket)
bit confused- in the howl function we send a run start, then the veto config update, then calls on produce messages to start a loop, send a load of fake run data then start another run- why not have the produce messages func start with sending run start, spew data and loop, means that we don't need the extra call to send_run_start in howl func.? |
|
I've put what I mean in the last commit- not tested it but would this work in theory? |
yes - i think that makes sense to me. I think you generally want a loop of send run start (if |
|
one thing that's important is that the job id of the run start and stop need to match. PS will review this properly soon - had a bit of time to look initially |
No problem thanks for the comments 👌 |
This branch has not been deployed
3 participants
Allows howl to send a vc00 containing veto info and also correctly passes veto info to event messages.
Removes functionality for passing just one probability and having all vetoes go off this.
Also does some refactoring as the main howl function was looking quite long.
To test I did different variations of
saluki howl sarada:9092 JackDTestwith different arguments e.g. names, enabled, probabilities. Look for results onsaluki consume sarada:9092/JackDTest_vetoConfig. Have a play and see if chosen arguments affect the result in an expected way.Also check that vc00 is sent after the run start as mentioned in ticket.