From 44a89cdf75bf72708e994bf0840bcfd752d90e11 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 16 Sep 2021 15:37:59 -0400 Subject: [PATCH 1/5] refactor: Change DeletedDatabase to DetailedDatabase So this info can be reused for active databases in detailed database lists. --- ...leted_database.rs => detailed_database.rs} | 9 +++-- data_types/src/lib.rs | 2 +- .../iox/management/v1/service.proto | 12 +++--- generated_types/src/deleted_database.rs | 18 --------- generated_types/src/detailed_database.rs | 18 +++++++++ generated_types/src/lib.rs | 2 +- influxdb_iox_client/src/client/management.rs | 2 +- iox_object_store/src/lib.rs | 39 +++++++++---------- server/src/database.rs | 2 +- server/src/lib.rs | 4 +- 10 files changed, 54 insertions(+), 54 deletions(-) rename data_types/src/{deleted_database.rs => detailed_database.rs} (70%) delete mode 100644 generated_types/src/deleted_database.rs create mode 100644 generated_types/src/detailed_database.rs diff --git a/data_types/src/deleted_database.rs b/data_types/src/detailed_database.rs similarity index 70% rename from data_types/src/deleted_database.rs rename to data_types/src/detailed_database.rs index 1c0d1f4574..be1a7d62dc 100644 --- a/data_types/src/deleted_database.rs +++ b/data_types/src/detailed_database.rs @@ -2,12 +2,15 @@ use crate::DatabaseName; use chrono::{DateTime, Utc}; use std::{fmt, str::FromStr}; -/// Metadata about a deleted database that could be restored or permanently deleted. +/// Detailed metadata about a database. #[derive(Debug, Clone, PartialEq)] -pub struct DeletedDatabase { +pub struct DetailedDatabase { + /// The name of the database pub name: DatabaseName<'static>, + /// The generation ID of the database in object storage pub generation_id: GenerationId, - pub deleted_at: DateTime, + /// The UTC datetime at which this database was deleted, if applicable + pub deleted_at: Option>, } /// Identifier for a generation of a particular database diff --git a/data_types/src/lib.rs b/data_types/src/lib.rs index 062af41bb8..f5858c9d56 100644 --- a/data_types/src/lib.rs +++ b/data_types/src/lib.rs @@ -14,7 +14,7 @@ pub mod chunk_metadata; pub mod consistent_hasher; mod database_name; pub mod database_rules; -pub mod deleted_database; +pub mod detailed_database; pub mod error; pub mod instant; pub mod job; diff --git a/generated_types/protos/influxdata/iox/management/v1/service.proto b/generated_types/protos/influxdata/iox/management/v1/service.proto index a6715fcbf2..fb527e66bc 100644 --- a/generated_types/protos/influxdata/iox/management/v1/service.proto +++ b/generated_types/protos/influxdata/iox/management/v1/service.proto @@ -189,18 +189,18 @@ message RestoreDatabaseResponse {} message ListDeletedDatabasesRequest {} message ListDeletedDatabasesResponse { - repeated DeletedDatabase deleted_databases = 1; + repeated DetailedDatabase deleted_databases = 1; } -// This resource represents a deleted database. -message DeletedDatabase { - // The generation ID of the deleted database. +// This resource represents detailed information about a database. +message DetailedDatabase { + // The generation ID of the database. uint64 generation_id = 1; - // The UTC datetime at which this database was deleted. + // The UTC datetime at which this database was deleted, if applicable. google.protobuf.Timestamp deleted_at = 2; - // The name of the deleted database. + // The name of the database. string db_name = 3; } diff --git a/generated_types/src/deleted_database.rs b/generated_types/src/deleted_database.rs deleted file mode 100644 index 7b485cb6e7..0000000000 --- a/generated_types/src/deleted_database.rs +++ /dev/null @@ -1,18 +0,0 @@ -use crate::influxdata::iox::management::v1 as management; -use data_types::deleted_database::DeletedDatabase; - -impl From for management::DeletedDatabase { - fn from(deleted: DeletedDatabase) -> Self { - let DeletedDatabase { - name, - generation_id, - deleted_at, - } = deleted; - - Self { - db_name: name.to_string(), - generation_id: generation_id.inner as u64, - deleted_at: Some(deleted_at.into()), - } - } -} diff --git a/generated_types/src/detailed_database.rs b/generated_types/src/detailed_database.rs new file mode 100644 index 0000000000..44b7d1cb1a --- /dev/null +++ b/generated_types/src/detailed_database.rs @@ -0,0 +1,18 @@ +use crate::influxdata::iox::management::v1 as management; +use data_types::detailed_database::DetailedDatabase; + +impl From for management::DetailedDatabase { + fn from(database: DetailedDatabase) -> Self { + let DetailedDatabase { + name, + generation_id, + deleted_at, + } = database; + + Self { + db_name: name.to_string(), + generation_id: generation_id.inner as u64, + deleted_at: deleted_at.map(Into::into), + } + } +} diff --git a/generated_types/src/lib.rs b/generated_types/src/lib.rs index 0176d23b84..84b38f56e9 100644 --- a/generated_types/src/lib.rs +++ b/generated_types/src/lib.rs @@ -143,7 +143,7 @@ pub use influxdata::platform::storage::*; pub mod chunk; pub mod database_rules; pub mod database_state; -pub mod deleted_database; +pub mod detailed_database; pub mod google; pub mod job; diff --git a/influxdb_iox_client/src/client/management.rs b/influxdb_iox_client/src/client/management.rs index 8a1ea54f9c..06836a3a57 100644 --- a/influxdb_iox_client/src/client/management.rs +++ b/influxdb_iox_client/src/client/management.rs @@ -604,7 +604,7 @@ impl Client { /// List deleted databases and metadata pub async fn list_deleted_databases( &mut self, - ) -> Result, ListDatabaseError> { + ) -> Result, ListDatabaseError> { let response = self .inner .list_deleted_databases(ListDeletedDatabasesRequest {}) diff --git a/iox_object_store/src/lib.rs b/iox_object_store/src/lib.rs index 28798ab1d6..eda7a05320 100644 --- a/iox_object_store/src/lib.rs +++ b/iox_object_store/src/lib.rs @@ -17,7 +17,7 @@ use bytes::{Bytes, BytesMut}; use chrono::{DateTime, Utc}; use data_types::{ - deleted_database::{DeletedDatabase, GenerationId}, + detailed_database::{DetailedDatabase, GenerationId}, error::ErrorLogger, server_id::ServerId, DatabaseName, @@ -96,19 +96,19 @@ pub struct IoxObjectStore { #[derive(Debug, Copy, Clone, PartialEq)] struct Generation { id: GenerationId, - deleted: Option>, + deleted_at: Option>, } impl Generation { fn active(id: usize) -> Self { Self { id: GenerationId { inner: id }, - deleted: None, + deleted_at: None, } } fn is_active(&self) -> bool { - self.deleted.is_none() + self.deleted_at.is_none() } } @@ -147,24 +147,21 @@ impl IoxObjectStore { pub async fn list_deleted_databases( inner: &ObjectStore, server_id: ServerId, - ) -> Result> { + ) -> Result> { let mut deleted_databases = vec![]; let all_dbs = Self::list_all_databases(inner, server_id).await; for (name, generations) in all_dbs? { - for deleted_gen in generations { - if let Generation { - id, - deleted: Some(deleted_at), - } = deleted_gen - { - deleted_databases.push(DeletedDatabase { - name: name.clone(), - generation_id: id, - deleted_at, - }); - } + for gen in generations + .into_iter() + .filter(|gen| gen.deleted_at.is_some()) + { + deleted_databases.push(DetailedDatabase { + name: name.clone(), + generation_id: gen.id, + deleted_at: gen.deleted_at, + }); } } @@ -228,13 +225,13 @@ impl IoxObjectStore { let generation_list_result = inner.list_with_delimiter(&prefix).await?; let tombstone_file = TombstonePath::new_from_object_store_path(&prefix); - let deleted = generation_list_result + let deleted_at = generation_list_result .objects .into_iter() .find(|object| object.location == tombstone_file.inner) .map(|object| object.last_modified); - generations.push(Generation { id, deleted }); + generations.push(Generation { id, deleted_at }); } else { // Deliberately ignoring errors with parsing; if the directory isn't a usize, it's // not a valid database generation directory and we should skip it. @@ -965,7 +962,7 @@ mod tests { generations[0], Generation { id: GenerationId { inner: 0 }, - deleted: None, + deleted_at: None, } ); @@ -999,7 +996,7 @@ mod tests { generations[1], Generation { id: GenerationId { inner: 1 }, - deleted: None, + deleted_at: None, } ); } diff --git a/server/src/database.rs b/server/src/database.rs index 1cbbe5eee6..9dc1891a25 100644 --- a/server/src/database.rs +++ b/server/src/database.rs @@ -9,7 +9,7 @@ use crate::{ }; use chrono::{DateTime, Utc}; use data_types::{ - database_rules::WriteBufferDirection, deleted_database::GenerationId, server_id::ServerId, + database_rules::WriteBufferDirection, detailed_database::GenerationId, server_id::ServerId, DatabaseName, }; use futures::{ diff --git a/server/src/lib.rs b/server/src/lib.rs index 804aee59d2..c69a35c053 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -72,7 +72,7 @@ use async_trait::async_trait; use chrono::Utc; use data_types::{ database_rules::{NodeGroup, RoutingRules, ShardId, Sink}, - deleted_database::DeletedDatabase, + detailed_database::DetailedDatabase, error::ErrorLogger, job::Job, server_id::ServerId, @@ -726,7 +726,7 @@ where } /// List all deleted databases in object storage. - pub async fn list_deleted_databases(&self) -> Result> { + pub async fn list_deleted_databases(&self) -> Result> { let server_id = { let state = self.shared.state.read(); let initialized = state.initialized()?; From 423a976744a74262f0136290693b919c38c5e368 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 16 Sep 2021 15:50:42 -0400 Subject: [PATCH 2/5] refactor: Use iterator adaptors rather than for loops --- iox_object_store/src/lib.rs | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/iox_object_store/src/lib.rs b/iox_object_store/src/lib.rs index eda7a05320..6dc9435594 100644 --- a/iox_object_store/src/lib.rs +++ b/iox_object_store/src/lib.rs @@ -148,24 +148,21 @@ impl IoxObjectStore { inner: &ObjectStore, server_id: ServerId, ) -> Result> { - let mut deleted_databases = vec![]; - - let all_dbs = Self::list_all_databases(inner, server_id).await; - - for (name, generations) in all_dbs? { - for gen in generations - .into_iter() - .filter(|gen| gen.deleted_at.is_some()) - { - deleted_databases.push(DetailedDatabase { - name: name.clone(), - generation_id: gen.id, - deleted_at: gen.deleted_at, - }); - } - } - - Ok(deleted_databases) + Ok(Self::list_all_databases(inner, server_id) + .await? + .into_iter() + .flat_map(|(name, generations)| { + let name = Arc::new(name); + generations.into_iter().filter_map(move |gen| { + let name = Arc::clone(&name); + gen.deleted_at.map(|_| DetailedDatabase { + name: (*name).clone(), + generation_id: gen.id, + deleted_at: gen.deleted_at, + }) + }) + }) + .collect()) } /// List database names in object storage along with all existing generations for each database From 51a40b31bffd38bbdf47e58fe7fef35994b2d516 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Fri, 17 Sep 2021 09:53:37 -0400 Subject: [PATCH 3/5] feat: Add a --detailed option to the database list CLI That will list both active and deleted databases with their generations. Closes #2462. --- .../iox/management/v1/service.proto | 9 ++ influxdb_iox_client/src/client/management.rs | 15 +++ iox_object_store/src/lib.rs | 24 ++++ server/src/lib.rs | 21 +++- src/commands/database.rs | 23 ++-- src/influxdb_ioxd/rpc/management.rs | 16 +++ tests/end_to_end_cases/management_cli.rs | 116 +++++++++++++++++- 7 files changed, 210 insertions(+), 14 deletions(-) diff --git a/generated_types/protos/influxdata/iox/management/v1/service.proto b/generated_types/protos/influxdata/iox/management/v1/service.proto index fb527e66bc..c19b54aea0 100644 --- a/generated_types/protos/influxdata/iox/management/v1/service.proto +++ b/generated_types/protos/influxdata/iox/management/v1/service.proto @@ -42,6 +42,9 @@ service ManagementService { // List deleted databases and their metadata. rpc ListDeletedDatabases(ListDeletedDatabasesRequest) returns (ListDeletedDatabasesResponse); + // List all databases and their metadata. + rpc ListDetailedDatabases(ListDetailedDatabasesRequest) returns (ListDetailedDatabasesResponse); + // List chunks available on this database rpc ListChunks(ListChunksRequest) returns (ListChunksResponse); @@ -192,6 +195,12 @@ message ListDeletedDatabasesResponse { repeated DetailedDatabase deleted_databases = 1; } +message ListDetailedDatabasesRequest {} + +message ListDetailedDatabasesResponse { + repeated DetailedDatabase databases = 1; +} + // This resource represents detailed information about a database. message DetailedDatabase { // The generation ID of the database. diff --git a/influxdb_iox_client/src/client/management.rs b/influxdb_iox_client/src/client/management.rs index 06836a3a57..b47e9e2c37 100644 --- a/influxdb_iox_client/src/client/management.rs +++ b/influxdb_iox_client/src/client/management.rs @@ -616,6 +616,21 @@ impl Client { Ok(response.into_inner().deleted_databases) } + /// List all databases and detailed metadata + pub async fn list_detailed_databases( + &mut self, + ) -> Result, ListDatabaseError> { + let response = self + .inner + .list_detailed_databases(ListDetailedDatabasesRequest {}) + .await + .map_err(|status| match status.code() { + tonic::Code::Unavailable => ListDatabaseError::Unavailable(status), + _ => ListDatabaseError::ServerError(status), + })?; + Ok(response.into_inner().databases) + } + /// Get database configuration /// /// If `omit_defaults` is false, return the current configuration diff --git a/iox_object_store/src/lib.rs b/iox_object_store/src/lib.rs index 6dc9435594..a0b7d5d071 100644 --- a/iox_object_store/src/lib.rs +++ b/iox_object_store/src/lib.rs @@ -165,6 +165,30 @@ impl IoxObjectStore { .collect()) } + /// List all databases in in object storage along with their generation IDs and if/when they + /// were deleted. Useful for visibility into object storage and finding databases to restore or + /// permanently delete. + pub async fn list_detailed_databases( + inner: &ObjectStore, + server_id: ServerId, + ) -> Result> { + Ok(Self::list_all_databases(inner, server_id) + .await? + .into_iter() + .flat_map(|(name, generations)| { + let name = Arc::new(name); + generations.into_iter().map(move |gen| { + let name = Arc::clone(&name); + DetailedDatabase { + name: (*name).clone(), + generation_id: gen.id, + deleted_at: gen.deleted_at, + } + }) + }) + .collect()) + } + /// List database names in object storage along with all existing generations for each database /// and whether the generations are marked as deleted or not. Useful for finding candidates /// to restore or to permanently delete. Makes many more calls to object storage than diff --git a/server/src/lib.rs b/server/src/lib.rs index c69a35c053..733539290f 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -241,6 +241,9 @@ pub enum Error { #[snafu(display("error listing deleted databases in object storage: {}", source))] ListDeletedDatabases { source: object_store::Error }, + + #[snafu(display("error listing detailed databases in object storage: {}", source))] + ListDetailedDatabases { source: object_store::Error }, } pub type Result = std::result::Result; @@ -725,7 +728,7 @@ where Ok(()) } - /// List all deleted databases in object storage. + /// List deleted databases in object storage. pub async fn list_deleted_databases(&self) -> Result> { let server_id = { let state = self.shared.state.read(); @@ -741,6 +744,22 @@ where .context(ListDeletedDatabases)?) } + /// List all databases, active and deleted, in object storage, including their generation IDs. + pub async fn list_detailed_databases(&self) -> Result> { + let server_id = { + let state = self.shared.state.read(); + let initialized = state.initialized()?; + initialized.server_id + }; + + Ok(IoxObjectStore::list_detailed_databases( + self.shared.application.object_store(), + server_id, + ) + .await + .context(ListDetailedDatabases)?) + } + pub async fn write_pb(&self, database_batch: pb::DatabaseBatch) -> Result<()> { let db_name = DatabaseName::new(database_batch.database_name.as_str()) .context(InvalidDatabaseName)?; diff --git a/src/commands/database.rs b/src/commands/database.rs index d0dd7678c5..a9161484a0 100644 --- a/src/commands/database.rs +++ b/src/commands/database.rs @@ -141,6 +141,11 @@ struct List { /// Whether to list databases marked as deleted instead, to restore or permanently delete. #[structopt(long)] deleted: bool, + + /// Whether to list detailed information, including generation IDs, about all databases, + /// whether they are active or marked as deleted. + #[structopt(long)] + detailed: bool, } /// Return configuration of specific database @@ -258,20 +263,24 @@ pub async fn command(connection: Connection, config: Config) -> Result<()> { } Command::List(list) => { let mut client = management::Client::new(connection); - if list.deleted { - let deleted = client.list_deleted_databases().await?; - println!("Deleted at | Generation ID | Name"); - println!("--------------------------------+---------------+--------"); - for database in deleted { + if list.deleted || list.detailed { + let databases = if list.deleted { + client.list_deleted_databases().await? + } else { + client.list_detailed_databases().await? + }; + println!("Deleted at | Generation ID | Name"); + println!("---------------------------------+---------------+--------"); + for database in databases { let deleted_at = database .deleted_at .and_then(|t| { let dt: Result, _> = t.try_into(); dt.ok().map(|d| d.to_string()) }) - .unwrap_or_else(|| String::from("Unknown")); + .unwrap_or_else(String::new); println!( - "{:<33}{:<16}{}", + "{:<34}{:<16}{}", deleted_at, database.generation_id, database.db_name, ); } diff --git a/src/influxdb_ioxd/rpc/management.rs b/src/influxdb_ioxd/rpc/management.rs index e9ac840c8d..5df9be086d 100644 --- a/src/influxdb_ioxd/rpc/management.rs +++ b/src/influxdb_ioxd/rpc/management.rs @@ -213,6 +213,22 @@ where })) } + async fn list_detailed_databases( + &self, + _: Request, + ) -> Result, Status> { + let databases = self + .server + .list_detailed_databases() + .await + .map_err(default_server_error_handler)? + .into_iter() + .map(Into::into) + .collect(); + + Ok(Response::new(ListDetailedDatabasesResponse { databases })) + } + async fn list_chunks( &self, request: Request, diff --git a/tests/end_to_end_cases/management_cli.rs b/tests/end_to_end_cases/management_cli.rs index 8236398a34..6ecbb7bab3 100644 --- a/tests/end_to_end_cases/management_cli.rs +++ b/tests/end_to_end_cases/management_cli.rs @@ -183,6 +183,8 @@ async fn test_create_database_immutable() { .stdout(predicate::str::contains(r#""immutable": true"#)); } +const DELETED_DB_DATETIME: &str = r#"[\d-]+\s[\d:\.]+\s[A-Z]+"#; + #[tokio::test] async fn delete_database() { let server_fixture = ServerFixture::create_shared().await; @@ -224,6 +226,18 @@ async fn delete_database() { .success() .stdout(predicate::str::contains(db).not()); + // Listing detailed database info does include the active database, along with its generation + Command::cargo_bin("influxdb_iox") + .unwrap() + .arg("database") + .arg("list") + .arg("--detailed") + .arg("--host") + .arg(addr) + .assert() + .success() + .stdout(predicate::str::is_match(format!(r#"(?m)^\s+0\s+{}$"#, db)).unwrap()); + // Delete the database Command::cargo_bin("influxdb_iox") .unwrap() @@ -257,7 +271,25 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout(predicate::str::contains(db)); + .stdout( + predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) + .unwrap(), + ); + + // Listing detailed database info does include the deleted database + Command::cargo_bin("influxdb_iox") + .unwrap() + .arg("database") + .arg("list") + .arg("--detailed") + .arg("--host") + .arg(addr) + .assert() + .success() + .stdout( + predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) + .unwrap(), + ); // Deleting the database again is an error Command::cargo_bin("influxdb_iox") @@ -306,7 +338,26 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout(predicate::str::contains(db)); + .stdout( + predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) + .unwrap(), + ); + + // Listing detailed database info includes both active and deleted + Command::cargo_bin("influxdb_iox") + .unwrap() + .arg("database") + .arg("list") + .arg("--detailed") + .arg("--host") + .arg(addr) + .assert() + .success() + .stdout( + predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) + .unwrap() + .and(predicate::str::is_match(format!(r#"(?m)^\s+1\s+{}$"#, db)).unwrap()), + ); // Delete the 2nd database Command::cargo_bin("influxdb_iox") @@ -342,8 +393,37 @@ async fn delete_database() { .assert() .success() .stdout( - predicate::str::contains(format!("0 {}", db)) - .and(predicate::str::contains(format!("1 {}", db))), + predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) + .unwrap() + .and( + predicate::str::is_match(format!( + r#"(?m)^{}\s+1\s+{}$"#, + DELETED_DB_DATETIME, db + )) + .unwrap(), + ), + ); + + // Listing detailed database info includes both deleted generations + Command::cargo_bin("influxdb_iox") + .unwrap() + .arg("database") + .arg("list") + .arg("--detailed") + .arg("--host") + .arg(addr) + .assert() + .success() + .stdout( + predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) + .unwrap() + .and( + predicate::str::is_match(format!( + r#"(?m)^{}\s+1\s+{}$"#, + DELETED_DB_DATETIME, db + )) + .unwrap(), + ), ); // Restore generation 0 @@ -384,9 +464,33 @@ async fn delete_database() { .assert() .success() .stdout( - predicate::str::contains(format!("1 {}", db)) - .and(predicate::str::contains(format!("0 {}", db)).not()), + predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) + .unwrap().not() + .and( + predicate::str::is_match(format!( + r#"(?m)^{}\s+1\s+{}$"#, + DELETED_DB_DATETIME, db + )) + .unwrap(), + ), ); + + // Listing detailed database info includes both active and deleted + Command::cargo_bin("influxdb_iox") + .unwrap() + .arg("database") + .arg("list") + .arg("--detailed") + .arg("--host") + .arg(addr) + .assert() + .success() + .stdout( + predicate::str::is_match(format!(r#"(?m)^{}\s+1\s+{}$"#, DELETED_DB_DATETIME, db)) + .unwrap() + .and(predicate::str::is_match(format!(r#"(?m)^\s+0\s+{}$"#, db)).unwrap()), + ); + } #[tokio::test] From 6520985b5dfb0de5af3e2268305da55c52f83e67 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Fri, 17 Sep 2021 10:54:01 -0400 Subject: [PATCH 4/5] refactor: Make the test regexes more readable --- tests/end_to_end_cases/management_cli.rs | 78 +++++++----------------- 1 file changed, 21 insertions(+), 57 deletions(-) diff --git a/tests/end_to_end_cases/management_cli.rs b/tests/end_to_end_cases/management_cli.rs index 6ecbb7bab3..1e8840350c 100644 --- a/tests/end_to_end_cases/management_cli.rs +++ b/tests/end_to_end_cases/management_cli.rs @@ -185,6 +185,18 @@ async fn test_create_database_immutable() { const DELETED_DB_DATETIME: &str = r#"[\d-]+\s[\d:\.]+\s[A-Z]+"#; +fn deleted_db_match(db: &str, generation_id: usize) -> predicates::str::RegexPredicate { + predicate::str::is_match(format!( + r#"(?m)^{}\s+{}\s+{}$"#, + DELETED_DB_DATETIME, generation_id, db + )) + .unwrap() +} + +fn active_db_match(db: &str, generation_id: usize) -> predicates::str::RegexPredicate { + predicate::str::is_match(format!(r#"(?m)^\s+{}\s+{}$"#, generation_id, db)).unwrap() +} + #[tokio::test] async fn delete_database() { let server_fixture = ServerFixture::create_shared().await; @@ -236,7 +248,7 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout(predicate::str::is_match(format!(r#"(?m)^\s+0\s+{}$"#, db)).unwrap()); + .stdout(active_db_match(db, 0)); // Delete the database Command::cargo_bin("influxdb_iox") @@ -271,10 +283,7 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout( - predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) - .unwrap(), - ); + .stdout(deleted_db_match(db, 0)); // Listing detailed database info does include the deleted database Command::cargo_bin("influxdb_iox") @@ -286,10 +295,7 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout( - predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) - .unwrap(), - ); + .stdout(deleted_db_match(db, 0)); // Deleting the database again is an error Command::cargo_bin("influxdb_iox") @@ -338,10 +344,7 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout( - predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) - .unwrap(), - ); + .stdout(deleted_db_match(db, 0)); // Listing detailed database info includes both active and deleted Command::cargo_bin("influxdb_iox") @@ -353,11 +356,7 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout( - predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) - .unwrap() - .and(predicate::str::is_match(format!(r#"(?m)^\s+1\s+{}$"#, db)).unwrap()), - ); + .stdout(deleted_db_match(db, 0).and(active_db_match(db, 1))); // Delete the 2nd database Command::cargo_bin("influxdb_iox") @@ -392,17 +391,7 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout( - predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) - .unwrap() - .and( - predicate::str::is_match(format!( - r#"(?m)^{}\s+1\s+{}$"#, - DELETED_DB_DATETIME, db - )) - .unwrap(), - ), - ); + .stdout(deleted_db_match(db, 0).and(deleted_db_match(db, 1))); // Listing detailed database info includes both deleted generations Command::cargo_bin("influxdb_iox") @@ -414,17 +403,7 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout( - predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) - .unwrap() - .and( - predicate::str::is_match(format!( - r#"(?m)^{}\s+1\s+{}$"#, - DELETED_DB_DATETIME, db - )) - .unwrap(), - ), - ); + .stdout(deleted_db_match(db, 0).and(deleted_db_match(db, 1))); // Restore generation 0 Command::cargo_bin("influxdb_iox") @@ -463,17 +442,7 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout( - predicate::str::is_match(format!(r#"(?m)^{}\s+0\s+{}$"#, DELETED_DB_DATETIME, db)) - .unwrap().not() - .and( - predicate::str::is_match(format!( - r#"(?m)^{}\s+1\s+{}$"#, - DELETED_DB_DATETIME, db - )) - .unwrap(), - ), - ); + .stdout(deleted_db_match(db, 0).not().and(deleted_db_match(db, 1))); // Listing detailed database info includes both active and deleted Command::cargo_bin("influxdb_iox") @@ -485,12 +454,7 @@ async fn delete_database() { .arg(addr) .assert() .success() - .stdout( - predicate::str::is_match(format!(r#"(?m)^{}\s+1\s+{}$"#, DELETED_DB_DATETIME, db)) - .unwrap() - .and(predicate::str::is_match(format!(r#"(?m)^\s+0\s+{}$"#, db)).unwrap()), - ); - + .stdout(active_db_match(db, 0).and(deleted_db_match(db, 1))); } #[tokio::test] From ac9c25d33c1ea8c759038f51ba8673abfc7a1c61 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Fri, 17 Sep 2021 15:55:37 -0400 Subject: [PATCH 5/5] fix: Use prettytable for detailed database output --- src/commands/database.rs | 23 +++++++++++++++++------ tests/end_to_end_cases/management_cli.rs | 8 ++++++-- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/commands/database.rs b/src/commands/database.rs index a9161484a0..36d961585f 100644 --- a/src/commands/database.rs +++ b/src/commands/database.rs @@ -11,6 +11,7 @@ use influxdb_iox_client::{ }, write::{self, WriteError}, }; +use prettytable::{format, Cell, Row, Table}; use std::{ convert::TryInto, fs::File, io::Read, num::NonZeroU64, path::PathBuf, str::FromStr, time::Duration, @@ -269,8 +270,15 @@ pub async fn command(connection: Connection, config: Config) -> Result<()> { } else { client.list_detailed_databases().await? }; - println!("Deleted at | Generation ID | Name"); - println!("---------------------------------+---------------+--------"); + + let mut table = Table::new(); + table.set_format(*format::consts::FORMAT_NO_LINESEP_WITH_TITLE); + table.set_titles(Row::new(vec![ + Cell::new("Deleted at"), + Cell::new("Generation ID"), + Cell::new("Name"), + ])); + for database in databases { let deleted_at = database .deleted_at @@ -279,11 +287,14 @@ pub async fn command(connection: Connection, config: Config) -> Result<()> { dt.ok().map(|d| d.to_string()) }) .unwrap_or_else(String::new); - println!( - "{:<34}{:<16}{}", - deleted_at, database.generation_id, database.db_name, - ); + table.add_row(Row::new(vec![ + Cell::new(&deleted_at), + Cell::new(&database.generation_id.to_string()), + Cell::new(&database.db_name), + ])); } + + print!("{}", table); } else { let names = client.list_database_names().await?; println!("{}", names.join("\n")) diff --git a/tests/end_to_end_cases/management_cli.rs b/tests/end_to_end_cases/management_cli.rs index 1e8840350c..72671eefe2 100644 --- a/tests/end_to_end_cases/management_cli.rs +++ b/tests/end_to_end_cases/management_cli.rs @@ -187,14 +187,18 @@ const DELETED_DB_DATETIME: &str = r#"[\d-]+\s[\d:\.]+\s[A-Z]+"#; fn deleted_db_match(db: &str, generation_id: usize) -> predicates::str::RegexPredicate { predicate::str::is_match(format!( - r#"(?m)^{}\s+{}\s+{}$"#, + r#"(?m)^\|\s+{}\s+\|\s+{}\s+\|\s+{}\s+\|$"#, DELETED_DB_DATETIME, generation_id, db )) .unwrap() } fn active_db_match(db: &str, generation_id: usize) -> predicates::str::RegexPredicate { - predicate::str::is_match(format!(r#"(?m)^\s+{}\s+{}$"#, generation_id, db)).unwrap() + predicate::str::is_match(format!( + r#"(?m)^\|\s+\|\s+{}\s+\|\s+{}\s+\|$"#, + generation_id, db + )) + .unwrap() } #[tokio::test]