From c14e67168163f99ea968aacf3ad70f3028e56e30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20R=2E=20Miguel?= Date: Tue, 30 Jan 2024 14:13:04 -0300 Subject: [PATCH 1/5] cli(context): perf. nits --- tembo-cli/src/cli/context.rs | 56 ++++++++++++------------------------ 1 file changed, 18 insertions(+), 38 deletions(-) diff --git a/tembo-cli/src/cli/context.rs b/tembo-cli/src/cli/context.rs index c18257471..0df7f0e53 100644 --- a/tembo-cli/src/cli/context.rs +++ b/tembo-cli/src/cli/context.rs @@ -1,7 +1,6 @@ use std::fs; -use anyhow::bail; -use anyhow::Ok; +use anyhow::{anyhow, bail, Ok}; use serde::Deserialize; use serde::Serialize; @@ -92,19 +91,11 @@ pub fn tembo_credentials_file_path() -> String { pub fn list_context() -> Result { let filename = tembo_context_file_path(); - let contents = match fs::read_to_string(filename.clone()) { - std::result::Result::Ok(c) => c, - Err(e) => { - bail!("Error reading file {filename}: {e}") - } - }; + let contents = fs::read_to_string(&filename) + .map_err(|err| anyhow!("Error reading file {filename}: {err}"))?; - let context: Context = match toml::from_str(&contents) { - std::result::Result::Ok(c) => c, - Err(e) => { - bail!("Issue with format of toml file {filename}: {e}") - } - }; + let context: Context = + toml::from_str(&contents).map_err(|err| anyhow!("Error reading file {filename}: {err}"))?; Ok(context) } @@ -112,42 +103,31 @@ pub fn list_context() -> Result { pub fn get_current_context() -> Result { let context = list_context()?; - let profiles = list_credentail_profiles()?; + let profiles = list_credential_profiles()?; - for mut e in context.environment { - if e.set.is_some() && e.set.unwrap() { - if e.profile.is_some() { - let credential = profiles - .iter() - .filter(|c| &c.name == e.profile.as_ref().unwrap()) - .last() - .unwrap(); + for mut env in context.environment { + if let Some(_is_set) = env.set { + if let Some(profile) = &env.profile { + let credential = profiles.iter().rev().find(|c| &c.name == profile).unwrap(); - e.selected_profile = Some(credential.to_owned()); + env.selected_profile = Some(credential.to_owned()); } - return Ok(e); + + return Ok(env); } } bail!("Tembo context not set"); } -pub fn list_credentail_profiles() -> Result, anyhow::Error> { +pub fn list_credential_profiles() -> Result, anyhow::Error> { let filename = tembo_credentials_file_path(); - let contents = match fs::read_to_string(filename.clone()) { - std::result::Result::Ok(c) => c, - Err(e) => { - bail!("Error reading file {filename}: {e}") - } - }; + let contents = fs::read_to_string(&filename) + .map_err(|err| anyhow!("Error reading file {filename}: {err}"))?; - let credential: Credential = match toml::from_str(&contents) { - std::result::Result::Ok(c) => c, - Err(e) => { - bail!("Issue with format of toml file {filename}: {e}") - } - }; + let credential: Credential = + toml::from_str(&contents).map_err(|err| anyhow!("Error reading file {filename}: {err}"))?; Ok(credential.profile) } From a4b7b245845ea61f642232549c5a596edcaf94ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20R=2E=20Miguel?= Date: Tue, 30 Jan 2024 14:31:18 -0300 Subject: [PATCH 2/5] cli: Env related minor refactor --- tembo-cli/src/cmd/apply.rs | 53 +++++++++++++++++++------------------ tembo-cli/src/cmd/delete.rs | 2 +- tembo-cli/src/cmd/logs.rs | 17 ++++++------ 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/tembo-cli/src/cmd/apply.rs b/tembo-cli/src/cmd/apply.rs index faea917d8..0b3ce72f7 100644 --- a/tembo-cli/src/cmd/apply.rs +++ b/tembo-cli/src/cmd/apply.rs @@ -99,7 +99,7 @@ fn tembo_cloud_apply( instance_settings: HashMap, ) -> Result<(), anyhow::Error> { for (_key, instance_setting) in instance_settings.iter() { - let result = tembo_cloud_apply_instance(env.clone(), instance_setting); + let result = tembo_cloud_apply_instance(&env, instance_setting); match result { Ok(i) => i, @@ -216,24 +216,23 @@ fn docker_apply_instance( } pub fn tembo_cloud_apply_instance( - env: Environment, + env: &Environment, instance_settings: &InstanceSettings, ) -> Result<(), anyhow::Error> { - let profile = env.clone().selected_profile.unwrap(); + let profile = env + .selected_profile + .as_ref() + .with_context(|| "Expected [environment] to have a selected profile")?; let config = Configuration { - base_path: profile.clone().tembo_host, - bearer_access_token: Some(profile.clone().tembo_access_token), + base_path: profile.tembo_host.clone(), + bearer_access_token: Some(profile.tembo_access_token.clone()), ..Default::default() }; - let mut instance_id = get_instance_id( - instance_settings.instance_name.clone(), - &config, - env.clone(), - )?; + let mut instance_id = get_instance_id(&instance_settings.instance_name, &config, env)?; - if let Some(env_instance_id) = instance_id.clone() { - update_existing_instance(env_instance_id, instance_settings, &config, env.clone()); + if let Some(env_instance_id) = &instance_id { + update_existing_instance(env_instance_id, instance_settings, &config, env); } else { let new_inst_req = create_new_instance(instance_settings, &config, env.clone()); match new_inst_req { @@ -254,7 +253,7 @@ pub fn tembo_cloud_apply_instance( sleep(Duration::from_secs(5)); let connection_info: Option> = - is_instance_up(instance_id.as_ref().unwrap().clone(), &config, &env)?; + is_instance_up(instance_id.as_ref().unwrap().clone(), &config, env)?; if connection_info.is_some() { let conn_info = get_conn_info_with_creds( @@ -322,9 +321,9 @@ fn get_conn_info_with_creds( } pub fn get_instance_id( - instance_name: String, + instance_name: &str, config: &Configuration, - env: Environment, + env: &Environment, ) -> Result, anyhow::Error> { let v = Runtime::new() .unwrap() @@ -372,17 +371,17 @@ pub fn is_instance_up( } fn update_existing_instance( - instance_id: String, + instance_id: &str, value: &InstanceSettings, config: &Configuration, - env: Environment, + env: &Environment, ) { let instance = get_update_instance(value); let v = Runtime::new().unwrap().block_on(put_instance( config, env.org_id.clone().unwrap().as_str(), - &instance_id, + instance_id, instance, )); @@ -503,12 +502,14 @@ fn get_postgres_config_cloud(instance_settings: &InstanceSettings) -> Vec>) -> Vec { +fn get_extensions( + maybe_extensions: Option>, +) -> Vec { let mut vec_extensions: Vec = vec![]; let mut vec_extension_location: Vec = vec![]; - if extensions.is_some() { - for (name, extension) in extensions.unwrap().iter() { + if let Some(extensions) = maybe_extensions { + for (name, extension) in extensions.into_iter() { vec_extension_location.push(ExtensionInstallLocation { database: None, schema: None, @@ -528,16 +529,16 @@ fn get_extensions(extensions: Option>) } fn get_trunk_installs( - extensions: Option>, + maybe_extensions: Option>, ) -> Vec { let mut vec_trunk_installs: Vec = vec![]; - if extensions.is_some() { - for (_, extension) in extensions.unwrap().iter() { + if let Some(extensions) = maybe_extensions { + for (_, extension) in extensions.into_iter() { if extension.trunk_project.is_some() { vec_trunk_installs.push(TrunkInstall { - name: extension.trunk_project.clone().unwrap(), - version: Some(extension.trunk_project_version.clone()), + name: extension.trunk_project.unwrap(), + version: Some(extension.trunk_project_version), }); } } diff --git a/tembo-cli/src/cmd/delete.rs b/tembo-cli/src/cmd/delete.rs index a1d99e5b0..809a6b6de 100644 --- a/tembo-cli/src/cmd/delete.rs +++ b/tembo-cli/src/cmd/delete.rs @@ -36,7 +36,7 @@ fn execute_tembo_cloud(env: Environment) -> Result<(), anyhow::Error> { }; for (_key, value) in instance_settings.iter() { - let instance_id = get_instance_id(value.instance_name.clone(), &config, env.clone())?; + let instance_id = get_instance_id(&value.instance_name, &config, &env)?; if let Some(env_instance_id) = instance_id { let v = Runtime::new().unwrap().block_on(delete_instance( &config, diff --git a/tembo-cli/src/cmd/logs.rs b/tembo-cli/src/cmd/logs.rs index 76b6a6623..72e6b9012 100644 --- a/tembo-cli/src/cmd/logs.rs +++ b/tembo-cli/src/cmd/logs.rs @@ -1,6 +1,6 @@ use crate::apply::{get_instance_id, get_instance_settings}; use crate::cli::context::{get_current_context, Target}; -use anyhow::{anyhow, Context, Result}; +use anyhow::{Context, Result}; use clap::Args; use reqwest::blocking::Client; use reqwest::header::{HeaderMap, HeaderValue, AUTHORIZATION}; @@ -28,8 +28,9 @@ struct LogEntry { } #[derive(Serialize, Deserialize, Debug)] +#[serde(rename = "camelCase")] struct LogResult { - resultType: String, + result_type: String, result: Vec, } @@ -74,11 +75,11 @@ pub fn execute() -> Result<()> { if env.target == Target::Docker.to_string() { let instance_settings = get_instance_settings(None, None)?; - for (instance_name, _settings) in instance_settings { + for (_instance_name, _settings) in instance_settings { docker_logs(&_settings.instance_name)?; } } else if env.target == Target::TemboCloud.to_string() { - cloud_logs(); + let _ = cloud_logs(); } Ok(()) } @@ -103,8 +104,7 @@ pub fn cloud_logs() -> Result<()> { ); for (_key, value) in instance_settings.iter() { - let instance_id_option = - get_instance_id(value.instance_name.clone(), &config, env.clone())?; + let instance_id_option = get_instance_id(&value.instance_name, &config, &env)?; let instance_id = if let Some(id) = instance_id_option { id @@ -161,8 +161,8 @@ pub fn docker_logs(instance_name: &str) -> Result<()> { #[cfg(test)] mod tests { use super::*; + use anyhow::anyhow; use assert_cmd::prelude::*; - use colorful::core::StrMarker; use std::env; use std::error::Error; use std::path::PathBuf; @@ -172,8 +172,7 @@ mod tests { #[tokio::test] async fn docker_logs() -> Result<(), Box> { - let root_dir = env!("CARGO_MANIFEST_DIR"); - let test_dir = PathBuf::from(root_dir).join("examples").join("set"); + let test_dir = PathBuf::from(ROOT_DIR).join("examples").join("set"); env::set_current_dir(&test_dir)?; From 1e52937af30fc76cb89152c05cea5203c3b6ee7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20R=2E=20Miguel?= Date: Tue, 30 Jan 2024 15:10:22 -0300 Subject: [PATCH 3/5] cli(fix): rename_all instead of rename --- tembo-cli/src/cmd/logs.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tembo-cli/src/cmd/logs.rs b/tembo-cli/src/cmd/logs.rs index 72e6b9012..7bf9f6ab9 100644 --- a/tembo-cli/src/cmd/logs.rs +++ b/tembo-cli/src/cmd/logs.rs @@ -28,7 +28,7 @@ struct LogEntry { } #[derive(Serialize, Deserialize, Debug)] -#[serde(rename = "camelCase")] +#[serde(rename_all = "camelCase")] struct LogResult { result_type: String, result: Vec, @@ -251,7 +251,6 @@ mod tests { #[tokio::test] async fn cloud_logs() { let valid_json_log = mock_query("valid_json").unwrap(); - let result = beautify_logs(&valid_json_log); - assert!(result.is_ok()); + beautify_logs(&valid_json_log).unwrap(); } } From 461d715a66442dee5545e644bdb18ad84b351cb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20R=2E=20Miguel?= Date: Tue, 30 Jan 2024 15:22:41 -0300 Subject: [PATCH 4/5] cli(validate): remove clone --- tembo-cli/src/cmd/validate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tembo-cli/src/cmd/validate.rs b/tembo-cli/src/cmd/validate.rs index 88f00b075..bb9ac98c5 100644 --- a/tembo-cli/src/cmd/validate.rs +++ b/tembo-cli/src/cmd/validate.rs @@ -43,7 +43,7 @@ pub fn execute(verbose: bool) -> Result<(), anyhow::Error> { let mut file_path = FileUtils::get_current_working_dir(); file_path.push_str("/tembo.toml"); - let contents = fs::read_to_string(file_path.clone())?; + let contents = fs::read_to_string(&file_path)?; let config: HashMap = toml::from_str(&contents)?; // Validate the config From 996ee7fd476db5900399ea7ad3fa24fda7a93cac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20R=2E=20Miguel?= Date: Tue, 30 Jan 2024 15:56:07 -0300 Subject: [PATCH 5/5] cli: update TOML deserialization error message --- tembo-cli/src/cli/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tembo-cli/src/cli/context.rs b/tembo-cli/src/cli/context.rs index 0df7f0e53..b5f76f971 100644 --- a/tembo-cli/src/cli/context.rs +++ b/tembo-cli/src/cli/context.rs @@ -127,7 +127,7 @@ pub fn list_credential_profiles() -> Result, anyhow::Error> { .map_err(|err| anyhow!("Error reading file {filename}: {err}"))?; let credential: Credential = - toml::from_str(&contents).map_err(|err| anyhow!("Error reading file {filename}: {err}"))?; + toml::from_str(&contents).map_err(|err| anyhow!("Issue with the format of the TOML file {filename}: {err}"))?; Ok(credential.profile) }