From 4cfcd3fc88760678f7db8b0ec53ef6e2fb2cc1fb Mon Sep 17 00:00:00 2001 From: Ryota Sakamoto Date: Tue, 21 May 2024 01:38:03 +0900 Subject: [PATCH] fix: fix review points Signed-off-by: Ryota Sakamoto --- src/app.rs | 14 +++++------ src/bootstrap.rs | 8 +++--- src/control.rs | 46 ++++++++++++++++++----------------- src/ddb/mod.rs | 1 + src/{util.rs => ddb/table.rs} | 2 +- src/main.rs | 2 +- src/parser.rs | 1 + src/transfer.rs | 6 ++--- tests/util/mod.rs | 7 ++++-- 9 files changed, 47 insertions(+), 40 deletions(-) create mode 100644 src/ddb/mod.rs rename src/{util.rs => ddb/table.rs} (99%) diff --git a/src/app.rs b/src/app.rs index 8cbd611..452a864 100644 --- a/src/app.rs +++ b/src/app.rs @@ -34,8 +34,8 @@ use tempfile::NamedTempFile; use thiserror::Error; use super::control; +use super::ddb::table; use super::key; -use super::util; /* ================================================= struct / enum / const @@ -59,7 +59,7 @@ pub struct TableSchema { pub pk: key::Key, pub sk: Option, pub indexes: Option>, - pub mode: util::Mode, + pub mode: table::Mode, } #[derive(Serialize, Deserialize, Debug, Clone)] @@ -323,8 +323,8 @@ impl Context { pub async fn effective_cache_key(&self) -> String { format!( "{}/{}", - &self.effective_region().await.as_ref(), - &self.effective_table_name() + self.effective_region().await.as_ref(), + self.effective_table_name() ) } @@ -515,7 +515,7 @@ pub async fn use_table( let tbl = tbl.clone(); let desc: TableDescription = control::describe_table_api(cx, tbl.clone()).await; save_using_target(cx, desc).await?; - println!("Now you're using the table '{}' ({}).", tbl, &cx.effective_region().await.as_ref()); + println!("Now you're using the table '{}' ({}).", tbl, cx.effective_region().await.as_ref()); }, None => bye(1, "You have to specify a table. How to use (1). 'dy use --table mytable', or (2) 'dy use mytable'."), }; @@ -561,7 +561,7 @@ pub async fn insert_to_table_cache( pk: key::typed_key("HASH", &desc).expect("pk should exist"), sk: key::typed_key("RANGE", &desc), indexes: index_schemas(&desc), - mode: util::extract_mode(&desc.billing_mode_summary), + mode: table::extract_mode(&desc.billing_mode_summary), }, ); cache.tables = Some(table_schema_hashmap); @@ -603,7 +603,7 @@ pub async fn table_schema(cx: &Context) -> TableSchema { pk: key::typed_key("HASH", &desc).expect("pk should exist"), sk: key::typed_key("RANGE", &desc), indexes: index_schemas(&desc), - mode: util::extract_mode(&desc.billing_mode_summary), + mode: table::extract_mode(&desc.billing_mode_summary), } } None => { diff --git a/src/bootstrap.rs b/src/bootstrap.rs index f5fa018..8867337 100644 --- a/src/bootstrap.rs +++ b/src/bootstrap.rs @@ -314,16 +314,16 @@ async fn prepare_table(cx: &app::Context, table_name: &str, keys: &[&str]) { Ok(desc) => { println!( "Started to create table '{}' in {} region. status: {}", - &table_name, - &cx.effective_region().await.as_ref(), + table_name, + cx.effective_region().await.as_ref(), desc.table_status.unwrap() ); } Err(e) => match e.into_service_error() { CreateTableError::ResourceInUseException(_) => println!( "[skip] Table '{}' already exists in {} region, skipping to create new one.", - &table_name, - &cx.effective_region().await.as_ref() + table_name, + cx.effective_region().await.as_ref() ), e => { debug!("CreateTable API call got an error -- {:#?}", e); diff --git a/src/control.rs b/src/control.rs index 069cf31..eed1bb6 100644 --- a/src/control.rs +++ b/src/control.rs @@ -35,7 +35,7 @@ use dialoguer::{theme::ColorfulTheme, Confirm, Select}; use tabwriter::TabWriter; use super::app; -use super::util; +use super::ddb::table; /* ================================================= Public functions @@ -108,7 +108,7 @@ pub async fn describe_all_tables(cx: app::Context) { } /// Executed when you call `$ dy desc (table)`. Retrieve TableDescription via describe_table_api function, -/// then print them in convenient way using util::print_table_description function (default/yaml). +/// then print them in convenient way using table::print_table_description function (default/yaml). pub async fn describe_table(cx: app::Context, target_table_to_desc: Option) { debug!("context: {:#?}", &cx); debug!("positional arg table name: {:?}", &target_table_to_desc); @@ -122,8 +122,8 @@ pub async fn describe_table(cx: app::Context, target_table_to_desc: Option { - util::print_table_description(new_context.effective_region().await.as_ref(), desc) + table::print_table_description(new_context.effective_region().await.as_ref(), desc) } // Some("raw") => println!("{:#?}", desc), Some(_) => { @@ -178,7 +178,7 @@ pub async fn create_table(cx: app::Context, name: String, given_keys: Vec util::print_table_description(cx.effective_region().await.as_ref(), desc), + Ok(desc) => table::print_table_description(cx.effective_region().await.as_ref(), desc), Err(e) => { debug!("CreateTable API call got an error -- {:#?}", e); error!("{}", e.into_service_error()); @@ -200,7 +200,8 @@ pub async fn create_table_api( &name, &given_keys ); - let (key_schema, attribute_definitions) = util::generate_essential_key_definitions(&given_keys); + let (key_schema, attribute_definitions) = + table::generate_essential_key_definitions(&given_keys); let config = cx.effective_sdk_config().await; let ddb = DynamoDbSdkClient::new(&config); @@ -230,7 +231,8 @@ pub async fn create_index(cx: app::Context, index_name: String, given_keys: Vec< &cx.effective_table_name() ); - let (key_schema, attribute_definitions) = util::generate_essential_key_definitions(&given_keys); + let (key_schema, attribute_definitions) = + table::generate_essential_key_definitions(&given_keys); let config = cx.effective_sdk_config().await; let ddb = DynamoDbSdkClient::new(&config); @@ -266,7 +268,7 @@ pub async fn create_index(cx: app::Context, index_name: String, given_keys: Vec< } Ok(res) => { debug!("Returned result: {:#?}", res); - util::print_table_description( + table::print_table_description( cx.effective_region().await.as_ref(), res.table_description.unwrap(), ); @@ -285,11 +287,11 @@ pub async fn update_table( let desc: TableDescription = describe_table_api(&cx, table_name_to_update.clone()).await; // Map given string into "Mode" enum. Note that in cmd.rs clap already limits acceptable values. - let switching_to_mode: Option = match mode_string { + let switching_to_mode: Option = match mode_string { None => None, Some(ms) => match ms.as_str() { - "provisioned" => Some(util::Mode::Provisioned), - "ondemand" => Some(util::Mode::OnDemand), + "provisioned" => Some(table::Mode::Provisioned), + "ondemand" => Some(table::Mode::OnDemand), _ => panic!("You shouldn't see this message as --mode can takes only 'provisioned' or 'ondemand'."), }, }; @@ -298,9 +300,9 @@ pub async fn update_table( let provisioned_throughput: Option = match &switching_to_mode { // when --mode is not given, no mode switch happens. Check the table's current mode. None => { - match util::extract_mode(&desc.clone().billing_mode_summary) { + match table::extract_mode(&desc.clone().billing_mode_summary) { // When currently OnDemand mode and you're not going to change the it, set None for CU. - util::Mode::OnDemand => { + table::Mode::OnDemand => { if wcu.is_some() || rcu.is_some() { println!("Ignoring --rcu/--wcu options as the table mode is OnDemand."); }; @@ -308,7 +310,7 @@ pub async fn update_table( } // When currently Provisioned mode and you're not going to change the it, // pass given rcu/wcu, and use current values if missing. Provisioned table should have valid capacity units so unwrap() here. - util::Mode::Provisioned => Some( + table::Mode::Provisioned => Some( ProvisionedThroughput::builder() .read_capacity_units(rcu.unwrap_or_else(|| { desc.clone() @@ -332,14 +334,14 @@ pub async fn update_table( // When the user trying to switch mode. Some(target_mode) => match target_mode { // when switching Provisioned->OnDemand mode, ProvisionedThroughput can be None. - util::Mode::OnDemand => { + table::Mode::OnDemand => { if wcu.is_some() || rcu.is_some() { println!("Ignoring --rcu/--wcu options as --mode ondemand."); }; None } // when switching OnDemand->Provisioned mode, set given wcu/rcu, fill with "5" as a default if not given. - util::Mode::Provisioned => Some( + table::Mode::Provisioned => Some( ProvisionedThroughput::builder() .read_capacity_units(rcu.unwrap_or(5)) .write_capacity_units(wcu.unwrap_or(5)) @@ -362,7 +364,7 @@ pub async fn update_table( ) .await { - Ok(desc) => util::print_table_description(cx.effective_region().await.as_ref(), desc), + Ok(desc) => table::print_table_description(cx.effective_region().await.as_ref(), desc), Err(e) => { debug!("UpdateTable API call got an error -- {:#?}", e); error!("{}", e.to_string()); @@ -385,7 +387,7 @@ pub async fn update_table( async fn update_table_api( cx: app::Context, table_name_to_update: String, - switching_to_mode: Option, + switching_to_mode: Option, provisioned_throughput: Option, ) -> Result< TableDescription, @@ -500,7 +502,7 @@ pub async fn list_backups(cx: app::Context, all_tables: bool) -> Result<(), IOEr .expect("status should exist") .as_str() .to_string(), - util::epoch_to_rfc3339( + table::epoch_to_rfc3339( backup .backup_creation_date_time .expect("creation date should exist") @@ -544,7 +546,7 @@ pub async fn restore(cx: app::Context, backup_name: Option, restore_name format!( "{} ({}, {} bytes)", b.to_owned().backup_name.unwrap(), - util::epoch_to_rfc3339(b.backup_creation_date_time.unwrap().as_secs_f64()), + table::epoch_to_rfc3339(b.backup_creation_date_time.unwrap().as_secs_f64()), b.backup_size_bytes.unwrap() ) }) @@ -594,7 +596,7 @@ pub async fn restore(cx: app::Context, backup_name: Option, restore_name debug!("Returned result: {:#?}", res); println!("Table restoration from: '{}' has been started", &backup_arn); let desc = res.table_description.unwrap(); - util::print_table_description(cx.effective_region().await.as_ref(), desc); + table::print_table_description(cx.effective_region().await.as_ref(), desc); } } } diff --git a/src/ddb/mod.rs b/src/ddb/mod.rs new file mode 100644 index 0000000..13971b0 --- /dev/null +++ b/src/ddb/mod.rs @@ -0,0 +1 @@ +pub mod table; diff --git a/src/util.rs b/src/ddb/table.rs similarity index 99% rename from src/util.rs rename to src/ddb/table.rs index 8511a52..8428c80 100644 --- a/src/util.rs +++ b/src/ddb/table.rs @@ -23,7 +23,7 @@ use aws_sdk_dynamodb::types::{ use chrono::DateTime; use log::error; -use super::key; +use crate::key; /* ================================================= struct / enum / const diff --git a/src/main.rs b/src/main.rs index 714e1ee..999f9d9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -32,11 +32,11 @@ mod bootstrap; mod cmd; mod control; mod data; +mod ddb; mod key; mod parser; mod shell; mod transfer; -mod util; /* ================================================= helper functions diff --git a/src/parser.rs b/src/parser.rs index febd82f..893a3f7 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1444,6 +1444,7 @@ impl DyneinParser { let result = GeneratedParser::parse(Rule::map_literal, exp); match result { Ok(mut pair) => { + // pair is parsed through Rule::map_literal so we expect that pair should be HashMap let item = parse_literal(pair.next().unwrap())? .convert_attribute_value() .as_m() diff --git a/src/transfer.rs b/src/transfer.rs index 0c71aa9..593a82c 100644 --- a/src/transfer.rs +++ b/src/transfer.rs @@ -37,7 +37,7 @@ use thiserror::Error; use super::app; use super::batch; use super::data; -use super::util; +use super::ddb::table; #[derive(Error, Debug)] pub enum DyneinExportError { @@ -151,7 +151,7 @@ pub async fn export( let ts: app::TableSchema = app::table_schema(&cx).await; let format_str: Option<&str> = format.as_deref(); - if ts.mode == util::Mode::Provisioned { + if ts.mode == table::Mode::Provisioned { let msg = "WARN: For the best performance on import/export, dynein recommends OnDemand mode. However the target table is Provisioned mode now. Proceed anyway?"; if !Confirm::new().with_prompt(msg).interact()? { app::bye(0, "Operation has been cancelled."); @@ -297,7 +297,7 @@ pub async fn import( let format_str: Option<&str> = format.as_deref(); let ts: app::TableSchema = app::table_schema(&cx).await; - if ts.mode == util::Mode::Provisioned { + if ts.mode == table::Mode::Provisioned { let msg = "WARN: For the best performance on import/export, dynein recommends OnDemand mode. However the target table is Provisioned mode now. Proceed anyway?"; if !Confirm::new().with_prompt(msg).interact()? { println!("Operation has been cancelled."); diff --git a/tests/util/mod.rs b/tests/util/mod.rs index b6a9f04..2395ee6 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -19,7 +19,7 @@ use std::env; use std::process::Command; // Run programs // use assert_cmd::cmd::Command; // Run programs - it seems to be equal to "use assert_cmd::prelude::* + use std::process::Command" -use aws_config::{Region, SdkConfig}; +use aws_config::{BehaviorVersion, Region, SdkConfig}; use aws_sdk_dynamodb::Client as DynamoDbSdkClient; use once_cell::sync::Lazy; use rand::{distributions::Alphanumeric, Rng}; @@ -340,7 +340,10 @@ async fn setup_container(port: i32) -> Result<(), Box> { // Wait dynamodb-local // https://docs.aws.amazon.com/sdk-for-rust/latest/dg/dynamodb-local.html let config = aws_sdk_dynamodb::config::Builder::from( - &SdkConfig::builder().region(Region::new("local")).build(), + &SdkConfig::builder() + .region(Region::new("local")) + .behavior_version(BehaviorVersion::v2024_03_28()) + .build(), ) .endpoint_url(format!("http://localhost:{}", port)) .build();