make 'account' a global option, add it for job start and make it optional

Author: Panaetius

coman/src/cli.rs

@@ -9,6 +9,7 @@ use crate::{
 };
 
 #[derive(Subcommand, Debug)]
+#[allow(clippy::large_enum_variant)]
 pub enum CliCommands {
     #[clap(about = "Show version and config file locations")]
     Version,
@@ -20,6 +21,8 @@ pub enum CliCommands {
         system: Option<String>,
         #[clap(short, long, ignore_case=true, value_parser=clap::builder::PossibleValuesParser::new(ComputePlatform::VARIANTS).map(|s|s.parse::<ComputePlatform>().unwrap()),help = "override compute platform (one of 'hpc', 'ml' or 'cw')")]
         platform: Option<ComputePlatform>,
+        #[clap(short, long, help = "override compute account to use (project or user)")]
+        account: Option<String>,
     },
     #[clap(about = "Create a new project configuration file")]
     Init {
@@ -90,16 +93,17 @@ pub enum CscsFileCommands {
     List { path: PathBuf },
     #[clap(alias("dl"), about = "Download a remote file [aliases: dl]")]
     Download {
-        #[clap(short, long, help = "account/project to use")]
-        account: Option<String>,
+        #[clap(help = "The path in the cluster to download")]
         remote: PathBuf,
+        #[clap(help = "The local path to download the file to")]
         local: PathBuf,
     },
     #[clap(alias("ul"), about = "Upload a file to remote storage [aliases: ul]")]
     Upload {
-        #[clap(short, long, help = "account/project to use")]
-        account: Option<String>,
+        #[clap(help = "The local path to upload to the cluster")]
         local: PathBuf,
+
+        #[clap(help = "the path in the cluster to upload to")]
         remote: PathBuf,
     },
 }

coman/src/config.rs

@@ -41,7 +41,7 @@ pub struct CscsConfig {
     #[serde(default)]
     pub current_platform: ComputePlatform,
     #[serde(default)]
-    pub account: String,
+    pub account: Option<String>,
     #[serde(default)]
     pub sbatch_script_template: String,
     #[serde(default)]

coman/src/cscs/api_client.rs

@@ -381,6 +381,7 @@ impl CscsApi {
     pub async fn start_job(
         &self,
         system_name: &str,
+        account: Option<String>,
         name: &str,
         script_path: PathBuf,
         envvars: HashMap<String, String>,
@@ -390,6 +391,7 @@ impl CscsApi {
         let result = post_compute_system_job(
             &self.client,
             system_name,
+            account,
             name,
             None,
             Some(script_path),
@@ -472,15 +474,10 @@ impl CscsApi {
     pub async fn transfer_upload(
         &self,
         system_name: &str,
-        account: &str,
+        account: Option<String>,
         path: PathBuf,
         size: i64,
     ) -> Result<(i64, S3Upload)> {
-        if account.is_empty() {
-            return Err(eyre!(
-                "An account is required, set it in the config file or with the --account flag"
-            ));
-        }
         let job = post_filesystem_transfer_upload(&self.client, system_name, account, path, size)
             .await
             .wrap_err("couldn't upload file")?;
@@ -497,12 +494,12 @@ impl CscsApi {
             .wrap_err("couldn't download file")?;
         Ok(content)
     }
-    pub async fn transfer_download(&self, system_name: &str, account: &str, path: PathBuf) -> Result<(i64, Url)> {
-        if account.is_empty() {
-            return Err(eyre!(
-                "An account is required, set it in the config file or with the --account flag"
-            ));
-        }
+    pub async fn transfer_download(
+        &self,
+        system_name: &str,
+        account: Option<String>,
+        path: PathBuf,
+    ) -> Result<(i64, Url)> {
         let job = post_filesystem_transfer_download(&self.client, system_name, account, path)
             .await
             .wrap_err("couldn't transfer file")?;

coman/src/cscs/cli.rs

@@ -99,6 +99,7 @@ pub(crate) async fn cli_cscs_job_log(
     }
 }
 
+#[allow(clippy::too_many_arguments)]
 pub(crate) async fn cli_cscs_job_start(
     script_file: Option<PathBuf>,
     image: Option<DockerImageUrl>,
@@ -107,8 +108,9 @@ pub(crate) async fn cli_cscs_job_start(
     env: Vec<(String, String)>,
     system: Option<String>,
     platform: Option<ComputePlatform>,
+    account: Option<String>,
 ) -> Result<()> {
-    cscs_start_job(script_file, image, command, workdir, env, system, platform).await
+    cscs_start_job(script_file, image, command, workdir, env, system, platform, account).await
 }
 
 pub(crate) async fn cli_cscs_job_cancel(

coman/src/cscs/handlers.rs

@@ -146,6 +146,7 @@ pub async fn cscs_job_cancel(job_id: i64, system: Option<String>, platform: Opti
     }
 }
 
+#[allow(clippy::too_many_arguments)]
 pub async fn cscs_start_job(
     script_file: Option<PathBuf>,
     image: Option<DockerImageUrl>,
@@ -154,12 +155,14 @@ pub async fn cscs_start_job(
     env: Vec<(String, String)>,
     system: Option<String>,
     platform: Option<ComputePlatform>,
+    account: Option<String>,
 ) -> Result<()> {
     match get_access_token().await {
         Ok(access_token) => {
             let api_client = CscsApi::new(access_token.0, platform).unwrap();
             let config = Config::new().unwrap();
             let current_system = &system.unwrap_or(config.cscs.current_system);
+            let account = account.or(config.cscs.account);
             let user_info = api_client.get_userinfo(current_system).await?;
             let current_system_info = api_client.get_system(current_system).await?;
             let scratch = match current_system_info {
@@ -245,7 +248,7 @@ pub async fn cscs_start_job(
 
             // start job
             api_client
-                .start_job(current_system, &name, script_path, envvars)
+                .start_job(current_system, account, &name, script_path, envvars)
                 .await?;
             Ok(())
         }
@@ -295,9 +298,8 @@ pub async fn cscs_file_download(
                 Ok(None)
             } else {
                 // download via s3
-                let job_data = api_client
-                    .transfer_download(current_system, &account.unwrap_or(config.cscs.account), remote)
-                    .await?;
+                let account = account.or(config.cscs.account);
+                let job_data = api_client.transfer_download(current_system, account, remote).await?;
                 Ok(Some((job_data.0, job_data.1, size)))
             }
         }
@@ -333,13 +335,9 @@ pub async fn cscs_file_upload(
                 Ok(None)
             } else {
                 // upload via s3
+                let account = account.or(config.cscs.account);
                 let transfer_data = api_client
-                    .transfer_upload(
-                        &config.cscs.current_system,
-                        &account.unwrap_or(config.cscs.account),
-                        remote,
-                        size as i64,
-                    )
+                    .transfer_upload(&config.cscs.current_system, account, remote, size as i64)
                     .await?;
 
                 Ok(Some(transfer_data))

coman/src/main.rs

@@ -57,6 +57,7 @@ async fn main() -> Result<()> {
                 command: cscs_command,
                 system,
                 platform,
+                account,
             } => match cscs_command {
                 cli::CscsCommands::Login => cli_cscs_login().await?,
                 cli::CscsCommands::Job { command } => match command {
@@ -69,15 +70,17 @@ async fn main() -> Result<()> {
                         command,
                         workdir,
                         env,
-                    } => cli_cscs_job_start(script_file, image, command, workdir, env, system, platform).await?,
+                    } => {
+                        cli_cscs_job_start(script_file, image, command, workdir, env, system, platform, account).await?
+                    }
                     cli::CscsJobCommands::Cancel { job_id } => cli_cscs_job_cancel(job_id, system, platform).await?,
                 },
                 cli::CscsCommands::File { command } => match command {
                     cli::CscsFileCommands::List { path } => cli_cscs_file_list(path, system, platform).await?,
-                    cli::CscsFileCommands::Download { remote, local, account } => {
+                    cli::CscsFileCommands::Download { remote, local } => {
                         cli_cscs_file_download(remote, local, account, system, platform).await?
                     }
-                    cli::CscsFileCommands::Upload { local, remote, account } => {
+                    cli::CscsFileCommands::Upload { local, remote } => {
                         cli_cscs_file_upload(local, remote, account, system, platform).await?
                     }
                 },

firecrest_client/src/compute_api.rs

@@ -14,6 +14,7 @@ use crate::{
 pub async fn post_compute_system_job(
     client: &FirecrestClient,
     system_name: &str,
+    account: Option<String>,
     name: &str,
     script: Option<&str>,
     script_path: Option<PathBuf>,
@@ -37,6 +38,7 @@ pub async fn post_compute_system_job(
                 .map_err(|_| eyre!("couldn't convert working dir path"))?
                 .unwrap_or("/".to_owned()),
             env: Some(JobDescriptionModelEnv::Object(json!(envvars))),
+            account,
             ..Default::default()
         },
     };

firecrest_client/src/filesystem_api.rs

@@ -139,7 +139,7 @@ pub async fn post_filesystem_ops_upload(
 pub async fn post_filesystem_transfer_upload(
     client: &FirecrestClient,
     system_name: &str,
-    account: &str,
+    account: Option<String>,
     path: PathBuf,
     size: i64,
 ) -> Result<UploadFileResponse> {
@@ -150,7 +150,7 @@ pub async fn post_filesystem_transfer_upload(
             file_size: Some(size),
             ..Default::default()
         }),
-        account: Some(account.to_owned()),
+        account,
     };
     let body_json = serde_json::to_string(&body)?;
     let response = client
@@ -194,14 +194,14 @@ pub async fn get_filesystem_ops_download(client: &FirecrestClient, system_name:
 pub async fn post_filesystem_transfer_download(
     client: &FirecrestClient,
     system_name: &str,
-    account: &str,
+    account: Option<String>,
     path: PathBuf,
 ) -> Result<DownloadFileResponse> {
     let file_path = path.as_os_str().to_str().ok_or(eyre!("couldn't cast path to string"))?;
     let body = PostFileDownloadRequest {
         source_path: Some(file_path.to_owned()),
         transfer_directives: PostFileDownloadRequestTransferDirectives::S3(S3TransferRequest::default()),
-        account: Some(account.to_owned()),
+        account,
     };
     let body_json = serde_json::to_string(&body)?;
     let response = client

firecrest_client/src/types.rs

@@ -9,7 +9,7 @@
 //! AUTO-GENERATED CODE - DO NOT EDIT!
 //!
 //! FirecREST
-//! Source: /tmp/.tmpJ9quLA.json
+//! Source: /tmp/.tmp2nZAlD.json
 //! Version: 2.4.1
 //! Generated by `oas3-gen v0.21.1`
 //!