Skip to content

Commit

Permalink
Apply pedantic clippy rules.
Browse files Browse the repository at this point in the history
Also refactor progress bars to clean up code and provide durations in ms.
  • Loading branch information
joaander committed May 8, 2024
1 parent fce27ab commit 8d4129a
Show file tree
Hide file tree
Showing 25 changed files with 605 additions and 482 deletions.
2 changes: 2 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ repos:
- id: check
- id: clippy
args:
- --all-targets
- --all-features
- --
- -Dwarnings
- repo: https://github.com/pre-commit/pre-commit-hooks
Expand Down
2 changes: 0 additions & 2 deletions doc/src/developers/style.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,3 @@

**Row's** [pre-commit](https://pre-commit.com/) configuration applies style fixes
with `rustfmt` checks for common errors with `clippy`.

TODO: Investigate clippy configuration and see if more stringent rules can be applied.
402 changes: 201 additions & 201 deletions src/builtin.rs

Large diffs are not rendered by default.

25 changes: 9 additions & 16 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,14 @@ use log::trace;
use std::io;
use std::path::PathBuf;

use cluster::ClusterArgs;
use directories::DirectoriesArgs;
use launchers::LaunchersArgs;
use scan::ScanArgs;
use status::StatusArgs;
use submit::SubmitArgs;

#[derive(Parser, Debug)]
#[command(version, about, long_about = None, subcommand_required = true)]
pub struct Options {
#[command(subcommand)]
pub command: Option<Commands>,

#[command(flatten)]
pub global_options: GlobalOptions,
pub global: GlobalOptions,

#[command(flatten)]
pub verbose: Verbosity<WarnLevel>,
Expand Down Expand Up @@ -67,31 +60,31 @@ pub enum ColorMode {
}

#[derive(Subcommand, Debug)]
pub enum ShowArgs {
pub enum ShowCommands {
/// Show the current state of the workflow.
Status(StatusArgs),
Status(status::Arguments),

/// List directories in the workspace.
Directories(DirectoriesArgs),
Directories(directories::Arguments),

/// Show the cluster configuration.
Cluster(ClusterArgs),
Cluster(cluster::Arguments),

/// Show launcher configurations.
Launchers(LaunchersArgs),
Launchers(launchers::Arguments),
}

#[derive(Subcommand, Debug)]
pub enum Commands {
/// Show properties of the workspace.
#[command(subcommand)]
Show(ShowArgs),
Show(ShowCommands),

/// Scan the workspace for completed actions.
Scan(ScanArgs),
Scan(scan::Arguments),

/// Submit workflow actions to the scheduler.
Submit(SubmitArgs),
Submit(submit::Arguments),
}

/// Parse directories passed in on the command line.
Expand Down
10 changes: 5 additions & 5 deletions src/cli/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ use std::error::Error;
use std::io::Write;

use crate::cli::GlobalOptions;
use row::cluster::ClusterConfiguration;
use row::cluster;

#[derive(Args, Debug)]
pub struct ClusterArgs {
pub struct Arguments {
/// Show all clusters.
#[arg(long, group = "select", display_order = 0)]
all: bool,
Expand All @@ -22,13 +22,13 @@ pub struct ClusterArgs {
/// Print the cluster to stdout in toml format.
///
pub fn cluster<W: Write>(
options: GlobalOptions,
args: ClusterArgs,
options: &GlobalOptions,
args: &Arguments,
output: &mut W,
) -> Result<(), Box<dyn Error>> {
debug!("Showing clusters.");

let clusters = ClusterConfiguration::open()?;
let clusters = cluster::Configuration::open()?;

if args.all {
info!("All cluster configurations:");
Expand Down
18 changes: 9 additions & 9 deletions src/cli/directories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use row::project::Project;
use row::MultiProgressContainer;

#[derive(Args, Debug)]
pub struct DirectoriesArgs {
pub struct Arguments {
/// Select the action to scan (defaults to all).
action: String,

Expand All @@ -37,14 +37,14 @@ pub struct DirectoriesArgs {
/// Print a human-readable list of directories, their status, job ID, and value(s).
///
pub fn directories<W: Write>(
options: GlobalOptions,
args: DirectoriesArgs,
options: &GlobalOptions,
args: Arguments,
multi_progress: &mut MultiProgressContainer,
output: &mut W,
) -> Result<(), Box<dyn Error>> {
debug!("Showing directories.");

let mut project = Project::open(options.io_threads, options.cluster, multi_progress)?;
let mut project = Project::open(options.io_threads, &options.cluster, multi_progress)?;

let query_directories =
cli::parse_directories(args.directories, || Ok(project.state().list_directories()))?;
Expand Down Expand Up @@ -111,9 +111,9 @@ pub fn directories<W: Write>(
if let Some((cluster, job_id)) =
submitted.get(&action.name).and_then(|d| d.get(directory))
{
row.push(Item::new(format!("{}/{}", cluster, job_id), Style::new()));
row.push(Item::new(format!("{cluster}/{job_id}"), Style::new()));
} else {
row.push(Item::new("".into(), Style::new()));
row.push(Item::new(String::new(), Style::new()));
}
for pointer in &args.value {
let value = project.state().values()[directory]
Expand All @@ -131,11 +131,11 @@ pub fn directories<W: Write>(

if !args.no_separate_groups && group_idx != groups.len() - 1 {
let mut row = vec![
Item::new("".to_string(), Style::new()),
Item::new("".to_string(), Style::new()),
Item::new(String::new(), Style::new()),
Item::new(String::new(), Style::new()),
];
for _ in &args.value {
row.push(Item::new("".to_string(), Style::new()))
row.push(Item::new(String::new(), Style::new()));
}
table.items.push(row);
}
Expand Down
14 changes: 7 additions & 7 deletions src/cli/launchers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ use std::error::Error;
use std::io::Write;

use crate::cli::GlobalOptions;
use row::cluster::ClusterConfiguration;
use row::launcher::LauncherConfiguration;
use row::cluster;
use row::launcher;

#[derive(Args, Debug)]
pub struct LaunchersArgs {
pub struct Arguments {
/// Show all launchers.
#[arg(long, display_order = 0)]
all: bool,
Expand All @@ -19,13 +19,13 @@ pub struct LaunchersArgs {
/// Print the launchers to stdout in toml format.
///
pub fn launchers<W: Write>(
options: GlobalOptions,
args: LaunchersArgs,
options: &GlobalOptions,
args: &Arguments,
output: &mut W,
) -> Result<(), Box<dyn Error>> {
debug!("Showing launchers.");

let launchers = LauncherConfiguration::open()?;
let launchers = launcher::Configuration::open()?;

if args.all {
info!("All launcher configurations:");
Expand All @@ -35,7 +35,7 @@ pub fn launchers<W: Write>(
&toml::to_string_pretty(launchers.full_config())?
)?;
} else {
let clusters = ClusterConfiguration::open()?;
let clusters = cluster::Configuration::open()?;
let cluster = clusters.identify(options.cluster.as_deref())?;

info!("Launcher configurations for cluster '{}':", cluster.name);
Expand Down
6 changes: 3 additions & 3 deletions src/cli/scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use row::{
};

#[derive(Args, Debug)]
pub struct ScanArgs {
pub struct Arguments {
/// Select the action to scan (defaults to all).
#[arg(short, long, display_order = 0)]
action: Option<String>,
Expand All @@ -27,8 +27,8 @@ pub struct ScanArgs {
/// Write the resulting list of completed directories to a completion pack file.
///
pub fn scan(
options: GlobalOptions,
args: ScanArgs,
options: &GlobalOptions,
args: Arguments,
multi_progress: &mut MultiProgressContainer,
) -> Result<(), Box<dyn std::error::Error>> {
debug!("Scanning the workspace for completed actions.");
Expand Down
14 changes: 7 additions & 7 deletions src/cli/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use row::workflow::ResourceCost;
use row::MultiProgressContainer;

#[derive(Args, Debug)]
pub struct StatusArgs {
pub struct Arguments {
/// Select the actions to summarize with a wildcard pattern.
#[arg(short, long, value_name = "pattern", default_value_t=String::from("*"), display_order=0)]
action: String,
Expand All @@ -28,7 +28,7 @@ pub struct StatusArgs {
}

/// Format a status string for non-terminal outputs.
fn make_row(action_name: &str, status: &Status, cost: ResourceCost) -> Vec<Item> {
fn make_row(action_name: &str, status: &Status, cost: &ResourceCost) -> Vec<Item> {
let mut result = Vec::with_capacity(6);
result.push(Item::new(action_name.to_string(), Style::new().bold()));
result.push(
Expand Down Expand Up @@ -62,7 +62,7 @@ fn make_row(action_name: &str, status: &Status, cost: ResourceCost) -> Vec<Item>

if !cost.is_zero() {
result.push(
Item::new(format!("{}", cost), Style::new().italic().dim())
Item::new(format!("{cost}"), Style::new().italic().dim())
.with_alignment(Alignment::Right),
);
}
Expand All @@ -75,15 +75,15 @@ fn make_row(action_name: &str, status: &Status, cost: ResourceCost) -> Vec<Item>
/// Print a human-readable summary of the workflow.
///
pub fn status<W: Write>(
options: GlobalOptions,
args: StatusArgs,
options: &GlobalOptions,
args: Arguments,
multi_progress: &mut MultiProgressContainer,
output: &mut W,
) -> Result<(), Box<dyn Error>> {
debug!("Showing the workflow's status.");
let action_matcher = WildMatch::new(&args.action);

let mut project = Project::open(options.io_threads, options.cluster, multi_progress)?;
let mut project = Project::open(options.io_threads, &options.cluster, multi_progress)?;

let query_directories =
cli::parse_directories(args.directories, || Ok(project.state().list_directories()))?;
Expand Down Expand Up @@ -131,7 +131,7 @@ pub fn status<W: Write>(
cost = cost + action.resources.cost(group.len());
}

table.items.push(make_row(&action.name, &status, cost));
table.items.push(make_row(&action.name, &status, &cost));
}

if matching_action_count == 0 {
Expand Down
23 changes: 12 additions & 11 deletions src/cli/submit.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clap::Args;
use console::style;
use indicatif::{HumanCount, HumanDuration};
use indicatif::HumanCount;
use log::{debug, info, trace, warn};
use signal_hook::consts::{SIGINT, SIGTERM};
use signal_hook::flag;
Expand All @@ -14,12 +14,13 @@ use std::time::Instant;
use wildmatch::WildMatch;

use crate::cli::GlobalOptions;
use row::format::HumanDuration;
use row::project::Project;
use row::workflow::{Action, ResourceCost};
use row::MultiProgressContainer;

#[derive(Args, Debug)]
pub struct SubmitArgs {
pub struct Arguments {
/// Select the actions to summarize with a wildcard pattern.
#[arg(short, long, value_name = "pattern", default_value_t=String::from("*"), display_order=0)]
action: String,
Expand All @@ -42,16 +43,17 @@ pub struct SubmitArgs {

/// Submit workflow actions to the scheduler.
///
#[allow(clippy::too_many_lines)]
pub fn submit<W: Write>(
options: GlobalOptions,
args: SubmitArgs,
options: &GlobalOptions,
args: Arguments,
multi_progress: &mut MultiProgressContainer,
output: &mut W,
) -> Result<(), Box<dyn Error>> {
debug!("Submitting workflow actions to the scheduler.");
let action_matcher = WildMatch::new(&args.action);

let mut project = Project::open(options.io_threads, options.cluster, multi_progress)?;
let mut project = Project::open(options.io_threads, &options.cluster, multi_progress)?;

let query_directories = if args.directories.is_empty() {
project.state().list_directories()
Expand Down Expand Up @@ -114,7 +116,7 @@ pub fn submit<W: Write>(
if job_count == 1 { "job" } else { "jobs" },
cost,
action.name
)
);
}
total_cost = total_cost + cost;

Expand All @@ -140,7 +142,7 @@ pub fn submit<W: Write>(
info!("script {}/{}:", index + 1, action_directories.len());
let script = scheduler.make_script(action, directories)?;

write!(output, "{}", script)?;
write!(output, "{script}")?;
output.flush()?;
}
project.close(multi_progress)?;
Expand Down Expand Up @@ -174,7 +176,7 @@ pub fn submit<W: Write>(

if std::io::stdout().is_terminal() && !args.yes {
let mut input = String::new();
multi_progress.multi_progress.suspend(|| {
multi_progress.suspend(|| {
print!("Proceed? [Y/n]: ");
io::stdout().flush().expect("Can flush stdout");
io::stdin()
Expand All @@ -198,8 +200,7 @@ pub fn submit<W: Write>(
// stdin and stdout directly.
project.close(multi_progress)?;

multi_progress.progress_bars.clear();
multi_progress.multi_progress.clear().unwrap();
multi_progress.clear().unwrap();

// Install the Ctrl-C signal handler to gracefully kill spawned processes
// and save the pending scheduled job cache before exiting. Allow the user
Expand All @@ -226,7 +227,7 @@ pub fn submit<W: Write>(
.italic()
.to_string();
}
message += &format!(" ({}).", style(HumanDuration(instant.elapsed())).dim());
message += &format!(" ({:#}).", style(HumanDuration(instant.elapsed())).dim());
println!("{message}");

let result = scheduler.submit(
Expand Down
Loading

0 comments on commit 8d4129a

Please sign in to comment.