Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions coman/.config/config.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
update_check_interval_hours = 24

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a more descriptive comment explaining why this interval was chosen.

Suggested change
update_check_interval_hours = 24
update_check_interval_hours = 24 # Check for updates every 24 hours

#ai-review-inline


[cscs]
# check https://docs.cscs.ch/access/firecrest/#firecrest-deployment-on-alps for possible system and platform combinations
current_system = "daint" # what system/cluster to execute commands on

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment references external documentation; consider adding a brief explanation of what 'current_system' controls.

Suggested change
current_system = "daint" # what system/cluster to execute commands on
current_system = "daint" # Specifies the target cluster for command execution

#ai-review-inline

Expand Down
2 changes: 1 addition & 1 deletion coman/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "coman"
version = "0.8.7"
version = "0.8.8"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version bump from 0.8.7 to 0.8.8 should be documented in changelog or release notes.

#ai-review-inline

edition = "2024"
description = "Compute Manager for managing HPC compute"
authors = ["Ralf Grubenmann <ralf.grubenmann@sdsc.ethz.ch>"]
Expand Down
41 changes: 41 additions & 0 deletions coman/src/cli/app.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use std::{error::Error, path::PathBuf, str::FromStr, thread};

use chrono::{DateTime, Local, TimeDelta};
use clap::{Args, Command, Parser, Subcommand, ValueHint, builder::TypedValueParser};
use clap_complete::{ArgValueCompleter, CompletionCandidate, Generator, Shell, generate};
use color_eyre::{Report, Result};
use itertools::Itertools;
use self_update::cargo_crate_version;
use strum::VariantNames;
use tokio::sync::mpsc;

Expand Down Expand Up @@ -568,3 +570,42 @@ fn is_bare_string(value_str: &str) -> bool {
pub fn print_completions<G: Generator>(generator: G, cmd: &mut Command) {
generate(generator, cmd, cmd.get_name().to_string(), &mut std::io::stdout());
}

pub fn update() -> Result<()> {
let status = self_update::backends::github::Update::configure()
.repo_owner("SwissDataScienceCenter")
.repo_name("coman")
.bin_name("coman")
.bin_path_in_archive("coman")
.show_download_progress(true)
.current_version(cargo_crate_version!())
.build()?
.update()?;
if status.updated() {
println!("Successfully updated to version: `{}`", status.version());
} else {
println!("Already up to date at version: `{}`", status.version());
}
Ok(())
}

pub async fn check_update() -> Result<()> {
let config = Config::new()?;
let data_dir = get_data_dir();
let stamp_path = data_dir.join("selfupdate.stamp");
let Ok(update_stamp) = std::fs::read_to_string(&stamp_path) else {
std::fs::write(&stamp_path, Local::now().to_rfc3339()).unwrap();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using unwrap() can cause panic; prefer proper error handling with ? operator.

Suggested change
std::fs::write(&stamp_path, Local::now().to_rfc3339()).unwrap();
std::fs::write(&stamp_path, Local::now().to_rfc3339()).map_err(|e| Report::msg(format!("Failed to write stamp file: {}", e)))?;

#ai-review-inline

return Ok(());
};
let update_stamp = DateTime::parse_from_rfc3339(&update_stamp)?;

let now = Local::now();
if now.naive_local() - update_stamp.naive_local()
> TimeDelta::hours(config.values.update_check_interval_hours as i64)
{
println!("checking for updates");
update()?;
std::fs::write(&stamp_path, Local::now().to_rfc3339())?;
}
Ok(())
}
2 changes: 2 additions & 0 deletions coman/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ pub struct ComanConfig {
#[serde(default)]
pub coman_squash_path: Option<PathBuf>,
#[serde(default)]
pub update_check_interval_hours: u64,
#[serde(default)]
pub cscs: CscsConfig,
}

Expand Down
27 changes: 7 additions & 20 deletions coman/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use clap::{CommandFactory, Parser};
use clap_complete::CompleteEnv;
use color_eyre::Result;
use keyring::set_global_service_name;
use self_update::cargo_crate_version;
use tokio::{runtime::Handle, sync::mpsc};
use tuirealm::{
Application, EventListenerCfg, PollStrategy, Sub, SubClause, SubEventClause, Update,
Expand All @@ -22,7 +21,7 @@ use crate::{
cli::{
app::{
Cli, CliCommands, ConfigCommands, CscsCommands, CscsFileCommands, CscsJobCommands, CscsSystemCommands,
get_config, print_completions, set_config, version,
check_update, get_config, print_completions, set_config, update, version,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 'update' function to CLI commands, ensure it's properly implemented and tested.

Suggested change
check_update, get_config, print_completions, set_config, update, version,
check_update, get_config, print_completions, set_config, update, version,

#ai-review-inline

},
exec::cli_exec_command,
proxy::cli_proxy_command,
Expand Down Expand Up @@ -64,6 +63,12 @@ async fn main() -> Result<()> {
set_global_service_name(env!("CARGO_PKG_NAME"));
crate::logging::init()?;
CompleteEnv::with_factory(Cli::command).complete();

// check self-update
if let Err(e) = check_update().await {
println!("Couldn't check for updates: {}", e);
}

let args = Cli::parse();
match args.command {
Some(command) => match command {
Expand Down Expand Up @@ -379,21 +384,3 @@ fn popup_exclusion_clause() -> SubClause<Id> {
SubClause::IsMounted(Id::SystemSelectPopup),
])))
}

fn update() -> Result<()> {
let status = self_update::backends::github::Update::configure()
.repo_owner("SwissDataScienceCenter")
.repo_name("coman")
.bin_name("coman")
.bin_path_in_archive("coman")
.show_download_progress(true)
.current_version(cargo_crate_version!())
.build()?
.update()?;
if status.updated() {
println!("Successfully updated to version: `{}`", status.version());
} else {
println!("Already up to date at version: `{}`", status.version());
}
Ok(())
}