Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assorted minor refactors and performance nits #519

Merged
merged 5 commits into from
Jan 30, 2024
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
56 changes: 18 additions & 38 deletions tembo-cli/src/cli/context.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::fs;

use anyhow::bail;
use anyhow::Ok;
use anyhow::{anyhow, bail, Ok};
use serde::Deserialize;
use serde::Serialize;

Expand Down Expand Up @@ -92,62 +91,43 @@ pub fn tembo_credentials_file_path() -> String {
pub fn list_context() -> Result<Context, anyhow::Error> {
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)
}

pub fn get_current_context() -> Result<Environment, anyhow::Error> {
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() {
vrmiguel marked this conversation as resolved.
Show resolved Hide resolved
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();
Copy link
Member Author

Choose a reason for hiding this comment

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

Same idea as the previous code (iterate and find the last matching element) but starting from the end, doing less work overall


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<Vec<Profile>, anyhow::Error> {
pub fn list_credential_profiles() -> Result<Vec<Profile>, 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!("Issue with the format of the TOML file {filename}: {err}"))?;

Ok(credential.profile)
}
53 changes: 27 additions & 26 deletions tembo-cli/src/cmd/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ fn tembo_cloud_apply(
instance_settings: HashMap<String, InstanceSettings>,
) -> 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,
Expand Down Expand Up @@ -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 {
Expand All @@ -254,7 +253,7 @@ pub fn tembo_cloud_apply_instance(
sleep(Duration::from_secs(5));

let connection_info: Option<Box<ConnectionInfo>> =
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(
Expand Down Expand Up @@ -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<Option<String>, anyhow::Error> {
let v = Runtime::new()
.unwrap()
Expand Down Expand Up @@ -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,
));

Expand Down Expand Up @@ -503,12 +502,14 @@ fn get_postgres_config_cloud(instance_settings: &InstanceSettings) -> Vec<PgConf
pg_configs
}

fn get_extensions(extensions: Option<HashMap<String, tembo_config::Extension>>) -> Vec<Extension> {
fn get_extensions(
maybe_extensions: Option<HashMap<String, tembo_config::Extension>>,
) -> Vec<Extension> {
let mut vec_extensions: Vec<Extension> = vec![];
let mut vec_extension_location: Vec<ExtensionInstallLocation> = 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,
Expand All @@ -528,16 +529,16 @@ fn get_extensions(extensions: Option<HashMap<String, tembo_config::Extension>>)
}

fn get_trunk_installs(
extensions: Option<HashMap<String, tembo_config::Extension>>,
maybe_extensions: Option<HashMap<String, tembo_config::Extension>>,
) -> Vec<TrunkInstall> {
let mut vec_trunk_installs: Vec<TrunkInstall> = 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),
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion tembo-cli/src/cmd/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 9 additions & 11 deletions tembo-cli/src/cmd/logs.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -28,8 +28,9 @@ struct LogEntry {
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(rename_all = "camelCase")]
struct LogResult {
resultType: String,
result_type: String,
result: Vec<LogEntry>,
}

Expand Down Expand Up @@ -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(())
}
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -172,8 +172,7 @@ mod tests {

#[tokio::test]
async fn docker_logs() -> Result<(), Box<dyn Error>> {
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)?;

Expand Down Expand Up @@ -252,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();
}
}
2 changes: 1 addition & 1 deletion tembo-cli/src/cmd/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome!

let contents = fs::read_to_string(&file_path)?;
let config: HashMap<String, InstanceSettings> = toml::from_str(&contents)?;

// Validate the config
Expand Down
Loading