feat: harmony-cli v0.1 #8 #9
1159
Cargo.lock
generated
@@ -9,6 +9,7 @@ members = [
|
||||
"harmony_tui",
|
||||
"opnsense-config",
|
||||
"opnsense-config-xml",
|
||||
"harmony_cli",
|
||||
]
|
||||
|
||||
[workspace.package]
|
||||
|
||||
20
README.md
@@ -8,6 +8,26 @@ This will launch Harmony's minimalist terminal ui which embeds a few demo scores
|
||||
|
||||
Usage instructions will be displayed at the bottom of the TUI.
|
||||
|
||||
`cargo run --bin example-cli -- --help`
|
||||
|
||||
This is the harmony CLI, a minimal implementation
|
||||
|
||||
The current help text:
|
||||
|
||||
````
|
||||
Usage: example-cli [OPTIONS]
|
||||
|
||||
Options:
|
||||
-y, --yes Run score(s) or not
|
||||
-f, --filter <FILTER> Filter query
|
||||
-i, --interactive Run interactive TUI or not
|
||||
-a, --all Run all or nth, defaults to all
|
||||
-n, --number <NUMBER> Run nth matching, zero indexed [default: 0]
|
||||
-l, --list list scores, will also be affected by run filter
|
||||
|
|
||||
-h, --help Print help
|
||||
-V, --version Print version```
|
||||
|
||||
## Core architecture
|
||||
|
||||

|
||||
````
|
||||
|
||||
19
examples/cli/Cargo.toml
Normal file
@@ -0,0 +1,19 @@
|
||||
[package]
|
||||
name = "example-cli"
|
||||
edition = "2024"
|
||||
version.workspace = true
|
||||
readme.workspace = true
|
||||
license.workspace = true
|
||||
publish = false
|
||||
|
||||
[dependencies]
|
||||
harmony = { path = "../../harmony" }
|
||||
harmony_cli = { path = "../../harmony_cli" }
|
||||
harmony_types = { path = "../../harmony_types" }
|
||||
cidr = { workspace = true }
|
||||
tokio = { workspace = true }
|
||||
harmony_macros = { path = "../../harmony_macros" }
|
||||
log = { workspace = true }
|
||||
env_logger = { workspace = true }
|
||||
url = { workspace = true }
|
||||
assert_cmd = "2.0.16"
|
||||
38
examples/cli/src/main.rs
Normal file
@@ -0,0 +1,38 @@
|
||||
use harmony::{
|
||||
inventory::Inventory,
|
||||
maestro::Maestro,
|
||||
modules::dummy::{ErrorScore, PanicScore, SuccessScore},
|
||||
topology::HAClusterTopology,
|
||||
};
|
||||
|
||||
#[tokio::main]
|
||||
async fn main() {
|
||||
let inventory = Inventory::autoload();
|
||||
let topology = HAClusterTopology::autoload();
|
||||
let mut maestro = Maestro::new(inventory, topology);
|
||||
|
||||
maestro.register_all(vec![
|
||||
Box::new(SuccessScore {}),
|
||||
Box::new(ErrorScore {}),
|
||||
Box::new(PanicScore {}),
|
||||
]);
|
||||
harmony_cli::init(maestro, None).await.unwrap();
|
||||
}
|
||||
|
||||
use assert_cmd::Command;
|
||||
|
||||
#[test]
|
||||
fn test_example_success() {
|
||||
let mut cmd = Command::cargo_bin("example-cli").unwrap();
|
||||
let assert = cmd.args(&["--yes", "--filter", "SuccessScore"]).assert();
|
||||
|
||||
assert.success();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_example_fail() {
|
||||
let mut cmd_fail = Command::cargo_bin("example-cli").unwrap();
|
||||
let assert_fail = cmd_fail.args(&["--yes", "--filter", "ErrorScore"]).assert();
|
||||
|
||||
assert_fail.failure();
|
||||
}
|
||||
19
harmony_cli/Cargo.toml
Normal file
@@ -0,0 +1,19 @@
|
||||
[package]
|
||||
name = "harmony_cli"
|
||||
edition = "2024"
|
||||
version.workspace = true
|
||||
readme.workspace = true
|
||||
license.workspace = true
|
||||
|
||||
[dependencies]
|
||||
assert_cmd = "2.0.17"
|
||||
clap = { version = "4.5.35", features = ["derive"] }
|
||||
harmony = { path = "../harmony" }
|
||||
harmony_tui = { path = "../harmony_tui", optional = true }
|
||||
inquire = "0.7.5"
|
||||
tokio.workspace = true
|
||||
|
||||
|
||||
[features]
|
||||
default = ["tui"]
|
||||
tui = ["dep:harmony_tui"]
|
||||
318
harmony_cli/src/lib.rs
Normal file
@@ -0,0 +1,318 @@
|
||||
use clap::Parser;
|
||||
use clap::builder::ArgPredicate;
|
||||
use harmony;
|
||||
use harmony::{score::Score, topology::Topology};
|
||||
use inquire::Confirm;
|
||||
|
||||
#[cfg(feature = "tui")]
|
||||
use harmony_tui;
|
||||
|
||||
#[derive(Parser, Debug)]
|
||||
#[command(version, about, long_about = None)]
|
||||
pub struct Args {
|
||||
/// Run score(s) without prompt
|
||||
#[arg(short, long, default_value_t = false, conflicts_with = "interactive")]
|
||||
yes: bool,
|
||||
|
||||
/// Filter query
|
||||
#[arg(short, long, conflicts_with = "interactive")]
|
||||
filter: Option<String>,
|
||||
|
||||
/// Run interactive TUI or not
|
||||
#[arg(short, long, default_value_t = false)]
|
||||
interactive: bool,
|
||||
|
||||
/// Run all or nth, defaults to all
|
||||
#[arg(
|
||||
short,
|
||||
long,
|
||||
default_value_t = true,
|
||||
default_value_if("number", ArgPredicate::IsPresent, "false"),
|
||||
conflicts_with = "number",
|
||||
conflicts_with = "interactive"
|
||||
)]
|
||||
all: bool,
|
||||
|
||||
/// Run nth matching, zero indexed
|
||||
#[arg(short, long, default_value_t = 0, conflicts_with = "interactive")]
|
||||
number: usize,
|
||||
|
||||
/// list scores, will also be affected by run filter
|
||||
#[arg(short, long, default_value_t = false, conflicts_with = "interactive")]
|
||||
list: bool,
|
||||
}
|
||||
|
||||
fn maestro_scores_filter<T: Topology + std::fmt::Debug + Send + Sync + 'static>(
|
||||
maestro: &harmony::maestro::Maestro<T>,
|
||||
all: bool,
|
||||
filter: Option<String>,
|
||||
number: usize,
|
||||
) -> Vec<Box<dyn Score<T>>> {
|
||||
let scores = maestro.scores();
|
||||
let scores_read = scores.read().expect("Should be able to read scores");
|
||||
let mut scores_vec: Vec<Box<dyn Score<T>>> = match filter {
|
||||
|
johnride
commented
So much better. I think there are more Rust idiomatic ways to handle that with Slices or something but it's good enough :
Now though you should at least reverse the map and filter calls so scores don't all get cloned for nothing. In general you always filter your ensemble before processing. So much better. I think there are more Rust idiomatic ways to handle that with Slices or something but it's good enough :
- Clear, easy to read and understand
- Relatively flat, not too much nested ifs and fors
- Leveraging functional Rust, usually makes for more robust code easily
Now though you should at least reverse the map and filter calls so scores don't all get cloned for nothing.
In general you always filter your ensemble before processing.
|
||||
Some(f) => scores_read
|
||||
.iter()
|
||||
.filter(|s| s.name().contains(&f))
|
||||
.map(|s| s.clone_box())
|
||||
.collect(),
|
||||
None => scores_read.iter().map(|s| s.clone_box()).collect(),
|
||||
};
|
||||
|
||||
if !all {
|
||||
let score = scores_vec.get(number);
|
||||
match score {
|
||||
Some(s) => scores_vec = vec![s.clone_box()],
|
||||
None => return vec![],
|
||||
}
|
||||
};
|
||||
|
taha marked this conversation as resolved
Outdated
johnride
commented
Calling a function here that uses .iter().filter() and returns an option is more idiomatic rust (and more robust) than the Calling a function here that uses .iter().filter() and returns an option is more idiomatic rust (and more robust) than the `mut bool found` and `mut count` Just filter the array and them you can match `filtered.len()`
```rust
let filtered = scores_read_vec.iter().filter(...).collect();
match filtered.len() { // Not sure about .len, could be length or size
0 => todo!("Handle zero"),
1 => todo!("Handle one"),
_ => todo!("Handle more than one"),
}
```
|
||||
|
||||
return scores_vec;
|
||||
}
|
||||
|
||||
// TODO: consider adding doctest for this function
|
||||
|
johnride
commented
This function, though pretty simple, deserves a test. Could be a good place for a doctest. Not necessary though, just spilling my thoughts. This function, though pretty simple, deserves a test. Could be a good place for a doctest. Not necessary though, just spilling my thoughts.
|
||||
fn list_scores_with_index<T: Topology + std::fmt::Debug + Send + Sync + 'static>(
|
||||
scores_vec: &Vec<Box<dyn Score<T>>>,
|
||||
) -> String {
|
||||
let mut display_str = String::new();
|
||||
for (i, s) in scores_vec.iter().enumerate() {
|
||||
let name = s.name();
|
||||
display_str.push_str(&format!("\n{i}: {name}"));
|
||||
}
|
||||
return display_str;
|
||||
}
|
||||
|
||||
pub async fn init<T: Topology + std::fmt::Debug + Send + Sync + 'static>(
|
||||
maestro: harmony::maestro::Maestro<T>,
|
||||
args_struct: Option<Args>,
|
||||
) -> Result<(), Box<dyn std::error::Error>> {
|
||||
let args = match args_struct {
|
||||
Some(args) => args,
|
||||
None => Args::parse(),
|
||||
};
|
||||
|
||||
#[cfg(feature = "tui")]
|
||||
if args.interactive {
|
||||
return harmony_tui::init(maestro).await;
|
||||
}
|
||||
|
||||
#[cfg(not(feature = "tui"))]
|
||||
if args.interactive {
|
||||
return Err("Not compiled with interactive support".into());
|
||||
}
|
||||
|
||||
let scores_vec = maestro_scores_filter(&maestro, args.all, args.filter, args.number);
|
||||
|
||||
if scores_vec.len() == 0 {
|
||||
return Err("No score found".into());
|
||||
}
|
||||
|
||||
// if list option is specified, print filtered list and exit
|
||||
if args.list {
|
||||
println!("Available scores:");
|
||||
println!("{}", list_scores_with_index(&scores_vec));
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
|
johnride
commented
Either I don't understand it or it's an outdated comment, I think those types of comment explaining WHAT the code is doing are generally bad. Comments should explain WHY. If you need to express a WHAT, use a function. Either I don't understand it or it's an outdated comment, I think those types of comment explaining WHAT the code is doing are generally bad. Comments should explain WHY. If you need to express a WHAT, use a function.
https://news.ycombinator.com/item?id=8073620
|
||||
// prompt user if --yes is not specified
|
||||
if !args.yes {
|
||||
let confirmation = Confirm::new(
|
||||
format!(
|
||||
"This will run the following scores: {}\n",
|
||||
list_scores_with_index(&scores_vec)
|
||||
)
|
||||
.as_str(),
|
||||
)
|
||||
.with_default(true)
|
||||
.prompt()
|
||||
.expect("Unexpected prompt error");
|
||||
|
||||
if !confirmation {
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
|
||||
|
johnride
commented
Weird comment here again, does not relate directly to the code block. I think I'd rather not have it. Weird comment here again, does not relate directly to the code block. I think I'd rather not have it.
|
||||
// Run filtered scores
|
||||
for s in scores_vec {
|
||||
println!("Running: {}", s.name());
|
||||
maestro.interpret(s).await?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod test {
|
||||
use harmony::{
|
||||
inventory::Inventory,
|
||||
maestro::Maestro,
|
||||
modules::dummy::{ErrorScore, PanicScore, SuccessScore},
|
||||
topology::HAClusterTopology,
|
||||
};
|
||||
use harmony::{score::Score, topology::Topology};
|
||||
|
||||
fn init_test_maestro() -> Maestro<HAClusterTopology> {
|
||||
let inventory = Inventory::autoload();
|
||||
let topology = HAClusterTopology::autoload();
|
||||
let mut maestro = Maestro::new(inventory, topology);
|
||||
|
||||
maestro.register_all(vec![
|
||||
Box::new(SuccessScore {}),
|
||||
Box::new(ErrorScore {}),
|
||||
Box::new(PanicScore {}),
|
||||
]);
|
||||
|
||||
maestro
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_init_success_score() {
|
||||
let maestro = init_test_maestro();
|
||||
let res = crate::init(
|
||||
maestro,
|
||||
Some(crate::Args {
|
||||
yes: true,
|
||||
filter: Some("SuccessScore".to_owned()),
|
||||
interactive: false,
|
||||
all: true,
|
||||
number: 0,
|
||||
list: false,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(res.is_ok());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_init_error_score() {
|
||||
let maestro = init_test_maestro();
|
||||
|
||||
let res = crate::init(
|
||||
maestro,
|
||||
Some(crate::Args {
|
||||
yes: true,
|
||||
filter: Some("ErrorScore".to_owned()),
|
||||
interactive: false,
|
||||
all: true,
|
||||
number: 0,
|
||||
list: false,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(res.is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_init_number_score() {
|
||||
let maestro = init_test_maestro();
|
||||
|
||||
let res = crate::init(
|
||||
maestro,
|
||||
Some(crate::Args {
|
||||
yes: true,
|
||||
filter: None,
|
||||
interactive: false,
|
||||
all: false,
|
||||
number: 0,
|
||||
list: false,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(res.is_ok());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_filter_fn_all() {
|
||||
let maestro = init_test_maestro();
|
||||
|
||||
let res = crate::maestro_scores_filter(&maestro, true, None, 0);
|
||||
|
||||
assert!(res.len() == 3);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_filter_fn_all_success() {
|
||||
let maestro = init_test_maestro();
|
||||
|
||||
let res = crate::maestro_scores_filter(&maestro, true, Some("Success".to_owned()), 0);
|
||||
|
||||
assert!(res.len() == 1);
|
||||
|
||||
assert!(
|
||||
maestro
|
||||
.interpret(res.get(0).unwrap().clone_box())
|
||||
.await
|
||||
.is_ok()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_filter_fn_all_error() {
|
||||
let maestro = init_test_maestro();
|
||||
|
||||
let res = crate::maestro_scores_filter(&maestro, true, Some("Error".to_owned()), 0);
|
||||
|
||||
assert!(res.len() == 1);
|
||||
|
||||
assert!(
|
||||
maestro
|
||||
.interpret(res.get(0).unwrap().clone_box())
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_filter_fn_all_score() {
|
||||
let maestro = init_test_maestro();
|
||||
|
||||
let res = crate::maestro_scores_filter(&maestro, true, Some("Score".to_owned()), 0);
|
||||
|
||||
assert!(res.len() == 3);
|
||||
|
||||
assert!(
|
||||
maestro
|
||||
.interpret(res.get(0).unwrap().clone_box())
|
||||
.await
|
||||
.is_ok()
|
||||
);
|
||||
assert!(
|
||||
maestro
|
||||
.interpret(res.get(1).unwrap().clone_box())
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_filter_fn_number() {
|
||||
let maestro = init_test_maestro();
|
||||
|
||||
let res = crate::maestro_scores_filter(&maestro, false, None, 0);
|
||||
|
||||
println!("{:#?}", res);
|
||||
|
||||
assert!(res.len() == 1);
|
||||
|
||||
assert!(
|
||||
maestro
|
||||
.interpret(res.get(0).unwrap().clone_box())
|
||||
.await
|
||||
.is_ok()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_filter_fn_number_invalid() {
|
||||
let maestro = init_test_maestro();
|
||||
|
||||
|
johnride
commented
Those 8 lines should be extracted to a utilty function in the test module such as "init_test_maestro() -> Maestro" Those 8 lines should be extracted to a utilty function in the test module such as "init_test_maestro() -> Maestro"
|
||||
let res = crate::maestro_scores_filter(&maestro, false, None, 11);
|
||||
|
||||
println!("{:#?}", res);
|
||||
|
||||
assert!(res.len() == 0);
|
||||
}
|
||||
}
|
||||
@@ -3,7 +3,7 @@ mod widget;
|
||||
use log::{debug, error, info};
|
||||
use tokio::sync::mpsc;
|
||||
use tokio_stream::StreamExt;
|
||||
use tui_logger::{TuiWidgetEvent, TuiWidgetState};
|
||||
use tui_logger::{TuiLoggerFile, TuiWidgetEvent, TuiWidgetState};
|
||||
use widget::{help::HelpWidget, score::ScoreListWidget};
|
||||
|
||||
use std::{panic, sync::Arc, time::Duration};
|
||||
@@ -123,7 +123,7 @@ impl<T: Topology + std::fmt::Debug + Send + Sync + 'static> HarmonyTUI<T> {
|
||||
// Set default level for unknown targets to Trace
|
||||
tui_logger::set_default_level(log::LevelFilter::Info);
|
||||
std::fs::create_dir_all("log")?;
|
||||
tui_logger::set_log_file("log/harmony.log").unwrap();
|
||||
tui_logger::set_log_file(TuiLoggerFile::new("log/harmony.log"));
|
||||
|
||||
color_eyre::install()?;
|
||||
let mut terminal = ratatui::init();
|
||||
|
||||
Je viens de l'essayer, semble pas pire, mais je pense que ca prend quelques trucs pour que le UX soit correct :