From 9dcd13d7a00b3cb244efb5baebc94ea4867df2ef Mon Sep 17 00:00:00 2001 From: Hubert Gruszecki Date: Mon, 17 Feb 2025 18:28:50 +0100 Subject: [PATCH] fix(cli): update CLI and tests for server default parameters This commit updates the CLI and associated tests to use "server_default" as the default value for max topic size and message expiry time, instead of "unlimited". This change ensures that the CLI uses the server's default configuration when parameters are not explicitly specified by the user. Additionally, the commit modifies the display and parsing logic for `IggyExpiry` to use "never_expire" instead of "none" for clarity. The tests have been updated to reflect these changes, ensuring that the new defaults are correctly applied and tested. This closes #1536. --- Cargo.lock | 2 +- cli/Cargo.toml | 2 +- cli/src/args/topic.rs | 24 ++++----- .../cli/topic/test_topic_create_command.rs | 32 ++++++----- .../cli/topic/test_topic_update_command.rs | 54 ++++++++++--------- sdk/src/cli/topics/update_topic.rs | 4 +- sdk/src/utils/expiry.rs | 6 +-- server/src/streaming/topics/topic.rs | 2 +- 8 files changed, 68 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4f4e3a800..0ccb4f16c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2503,7 +2503,7 @@ dependencies = [ [[package]] name = "iggy-cli" -version = "0.8.8" +version = "0.8.9" dependencies = [ "ahash 0.8.11", "anyhow", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 526a79f8d..c5fc54f0a 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "iggy-cli" -version = "0.8.8" +version = "0.8.9" edition = "2021" authors = ["bartosz.ciesla@gmail.com"] repository = "https://github.com/iggy-rs/iggy" diff --git a/cli/src/args/topic.rs b/cli/src/args/topic.rs index fb8659350..10f8bc1d6 100644 --- a/cli/src/args/topic.rs +++ b/cli/src/args/topic.rs @@ -97,19 +97,19 @@ pub(crate) struct TopicCreateArgs { /// Compression algorithm for the topic, set to "none" for no compression #[arg(value_parser = clap::value_parser!(CompressionAlgorithm), verbatim_doc_comment)] pub(crate) compression_algorithm: CompressionAlgorithm, - /// Max topic size + /// Max topic size in human-readable format like "unlimited" or "15GB" /// - /// ("unlimited" or skipping parameter disables max topic size functionality in topic) + /// "server_default" or skipping parameter makes CLI to use server default (from current server config) max topic size /// Can't be lower than segment size in the config. - #[arg(short, long, default_value = "unlimited", verbatim_doc_comment)] + #[arg(short, long, default_value = "server_default", verbatim_doc_comment)] pub(crate) max_topic_size: MaxTopicSize, /// Replication factor for the topic #[arg(short, long, default_value = "1")] pub(crate) replication_factor: u8, - /// Message expiry time in human-readable format like 15days 2min 2s + /// Message expiry time in human-readable format like "unlimited" or "15days 2min 2s" /// - /// ("unlimited" or skipping parameter disables message expiry functionality in topic) - #[arg(value_parser = clap::value_parser!(IggyExpiry), verbatim_doc_comment)] + /// "server_default" or skipping parameter makes CLI to use server default (from current server config) expiry time + #[arg(default_value = "server_default", value_parser = clap::value_parser!(IggyExpiry), verbatim_doc_comment)] pub(crate) message_expiry: Vec, } @@ -144,19 +144,19 @@ pub(crate) struct TopicUpdateArgs { /// Compression algorithm for the topic, set to "none" for no compression #[arg(value_parser = clap::value_parser!(CompressionAlgorithm), verbatim_doc_comment)] pub(crate) compression_algorithm: CompressionAlgorithm, - /// New max topic size + /// New max topic size in human-readable format like "unlimited" or "15GB" /// - /// ("unlimited" or skipping parameter causes removal of max topic size parameter in topic) + /// "server_default" or skipping parameter makes CLI to use server default (from current server config) max topic size /// Can't be lower than segment size in the config. - #[arg(short, long, default_value = "unlimited", verbatim_doc_comment)] + #[arg(short, long, default_value = "server_default", verbatim_doc_comment)] pub(crate) max_topic_size: MaxTopicSize, #[arg(short, long, default_value = "1")] /// New replication factor for the topic pub(crate) replication_factor: u8, - /// New message expiry time in human-readable format like 15days 2min 2s + /// New message expiry time in human-readable format like "unlimited" or "15days 2min 2s" /// - /// ("unlimited" or skipping parameter causes removal of expiry parameter in topic) - #[arg(value_parser = clap::value_parser!(IggyExpiry), verbatim_doc_comment)] + /// "server_default" or skipping parameter makes CLI to use server default (from current server config) expiry time + #[arg(default_value = "server_default", value_parser = clap::value_parser!(IggyExpiry), verbatim_doc_comment)] pub(crate) message_expiry: Vec, } diff --git a/integration/tests/cli/topic/test_topic_create_command.rs b/integration/tests/cli/topic/test_topic_create_command.rs index a8391371b..85b5f51d3 100644 --- a/integration/tests/cli/topic/test_topic_create_command.rs +++ b/integration/tests/cli/topic/test_topic_create_command.rs @@ -111,7 +111,7 @@ impl IggyCmdTestCase for TestTopicCreateCmd { let compression_algorithm = &self.compression_algorithm; let message_expiry = (match &self.message_expiry { Some(value) => value.join(" "), - None => IggyExpiry::NeverExpire.to_string(), + None => IggyExpiry::ServerDefault.to_string(), }) .to_string(); @@ -189,7 +189,7 @@ pub async fn should_be_successful() { 1, Default::default(), None, - MaxTopicSize::Unlimited, + MaxTopicSize::ServerDefault, 1, TestStreamId::Numeric, )) @@ -203,7 +203,7 @@ pub async fn should_be_successful() { 5, Default::default(), None, - MaxTopicSize::Unlimited, + MaxTopicSize::ServerDefault, 1, TestStreamId::Named, )) @@ -281,21 +281,23 @@ Arguments: Compression algorithm for the topic, set to "none" for no compression [MESSAGE_EXPIRY]... - Message expiry time in human-readable format like 15days 2min 2s + Message expiry time in human-readable format like "unlimited" or "15days 2min 2s" {CLAP_INDENT} - ("unlimited" or skipping parameter disables message expiry functionality in topic) + "server_default" or skipping parameter makes CLI to use server default (from current server config) expiry time +{CLAP_INDENT} + [default: server_default] Options: -t, --topic-id Topic ID to create -m, --max-topic-size - Max topic size + Max topic size in human-readable format like "unlimited" or "15GB" {CLAP_INDENT} - ("unlimited" or skipping parameter disables max topic size functionality in topic) + "server_default" or skipping parameter makes CLI to use server default (from current server config) max topic size Can't be lower than segment size in the config. {CLAP_INDENT} - [default: unlimited] + [default: server_default] -r, --replication-factor Replication factor for the topic @@ -327,13 +329,17 @@ Arguments: Name of the topic Number of partitions inside the topic Compression algorithm for the topic, set to "none" for no compression - [MESSAGE_EXPIRY]... Message expiry time in human-readable format like 15days 2min 2s + [MESSAGE_EXPIRY]... Message expiry time in human-readable format like "unlimited" or "15days 2min 2s" [default: server_default] Options: - -t, --topic-id Topic ID to create - -m, --max-topic-size Max topic size [default: unlimited] - -r, --replication-factor Replication factor for the topic [default: 1] - -h, --help Print help (see more with '--help') + -t, --topic-id + Topic ID to create + -m, --max-topic-size + Max topic size in human-readable format like "unlimited" or "15GB" [default: server_default] + -r, --replication-factor + Replication factor for the topic [default: 1] + -h, --help + Print help (see more with '--help') "#, ), )) diff --git a/integration/tests/cli/topic/test_topic_update_command.rs b/integration/tests/cli/topic/test_topic_update_command.rs index 9a60ece1f..bf387867f 100644 --- a/integration/tests/cli/topic/test_topic_update_command.rs +++ b/integration/tests/cli/topic/test_topic_update_command.rs @@ -84,10 +84,7 @@ impl TestTopicUpdateCmd { command.push(self.topic_new_name.clone()); command.push(self.topic_new_compression_algorithm.to_string()); - - if let MaxTopicSize::Custom(max_size) = &self.topic_new_max_size { - command.push(format!("--max-topic-size={}", max_size)); - } + command.push(format!("--max-topic-size={}", self.topic_new_max_size)); if self.topic_new_replication_factor != 1 { command.push(format!( @@ -113,7 +110,7 @@ impl IggyCmdTestCase for TestTopicUpdateCmd { assert!(stream.is_ok()); let message_expiry = match &self.message_expiry { - None => IggyExpiry::NeverExpire, + None => IggyExpiry::ServerDefault, Some(message_expiry) => { let duration: Duration = *message_expiry.join(" ").parse::().unwrap(); @@ -160,20 +157,20 @@ impl IggyCmdTestCase for TestTopicUpdateCmd { let message_expiry = (match &self.topic_new_message_expiry { Some(value) => value.join(" "), - None => IggyExpiry::NeverExpire.to_string(), + None => IggyExpiry::ServerDefault.to_string(), }) .to_string(); - let max_topic_size = self.topic_new_max_size.to_string(); - let replication_factor = self.topic_new_replication_factor; let new_topic_name = &self.topic_new_name; + let new_max_topic_size = self.topic_new_max_size.to_string(); let expected_message = format!("Executing update topic with ID: {topic_id}, name: {new_topic_name}, \ - message expiry: {message_expiry}, compression algorithm: {compression_algorithm}, max topic size: {max_topic_size}, \ + message expiry: {message_expiry}, compression algorithm: {compression_algorithm}, max topic size: {new_max_topic_size}, \ replication factor: {replication_factor}, in stream with ID: {stream_id}\n\ Topic with ID: {topic_id} updated name: {new_topic_name}, updated message expiry: {message_expiry}, \ - updated compression algorithm: {compression_algorithm} in stream with ID: {stream_id}\n"); + updated compression algorithm: {compression_algorithm}, updated max topic size: {new_max_topic_size}, \ + updated replication factor: {replication_factor} in stream with ID: {stream_id}\n"); command_state.success().stdout(diff(expected_message)); } @@ -233,7 +230,7 @@ pub async fn should_be_successful() { String::from("sync"), Default::default(), None, - MaxTopicSize::Unlimited, + MaxTopicSize::ServerDefault, 1, String::from("new_name"), CompressionAlgorithm::Gzip, @@ -252,7 +249,7 @@ pub async fn should_be_successful() { String::from("topic"), Default::default(), None, - MaxTopicSize::Unlimited, + MaxTopicSize::ServerDefault, 1, String::from("testing"), CompressionAlgorithm::Gzip, @@ -271,7 +268,7 @@ pub async fn should_be_successful() { String::from("development"), Default::default(), None, - MaxTopicSize::Unlimited, + MaxTopicSize::ServerDefault, 1, String::from("development"), CompressionAlgorithm::Gzip, @@ -290,7 +287,7 @@ pub async fn should_be_successful() { String::from("probe"), Default::default(), None, - MaxTopicSize::Unlimited, + MaxTopicSize::ServerDefault, 1, String::from("development"), CompressionAlgorithm::Gzip, @@ -314,12 +311,12 @@ pub async fn should_be_successful() { String::from("testing"), Default::default(), Some(vec![String::from("1s")]), - MaxTopicSize::Unlimited, + MaxTopicSize::ServerDefault, 1, String::from("testing"), CompressionAlgorithm::Gzip, Some(vec![String::from("1m 6s")]), - MaxTopicSize::Unlimited, + MaxTopicSize::ServerDefault, 1, TestStreamId::Numeric, TestTopicId::Numeric, @@ -337,7 +334,7 @@ pub async fn should_be_successful() { String::from("1m"), String::from("1h"), ]), - MaxTopicSize::Unlimited, + MaxTopicSize::ServerDefault, 1, String::from("testing"), CompressionAlgorithm::Gzip, @@ -391,18 +388,20 @@ Arguments: Compression algorithm for the topic, set to "none" for no compression [MESSAGE_EXPIRY]... - New message expiry time in human-readable format like 15days 2min 2s + New message expiry time in human-readable format like "unlimited" or "15days 2min 2s" +{CLAP_INDENT} + "server_default" or skipping parameter makes CLI to use server default (from current server config) expiry time {CLAP_INDENT} - ("unlimited" or skipping parameter causes removal of expiry parameter in topic) + [default: server_default] Options: -m, --max-topic-size - New max topic size + New max topic size in human-readable format like "unlimited" or "15GB" {CLAP_INDENT} - ("unlimited" or skipping parameter causes removal of max topic size parameter in topic) + "server_default" or skipping parameter makes CLI to use server default (from current server config) max topic size Can't be lower than segment size in the config. {CLAP_INDENT} - [default: unlimited] + [default: server_default] -r, --replication-factor New replication factor for the topic @@ -435,12 +434,15 @@ Arguments: Topic ID to update New name for the topic Compression algorithm for the topic, set to "none" for no compression - [MESSAGE_EXPIRY]... New message expiry time in human-readable format like 15days 2min 2s + [MESSAGE_EXPIRY]... New message expiry time in human-readable format like "unlimited" or "15days 2min 2s" [default: server_default] Options: - -m, --max-topic-size New max topic size [default: unlimited] - -r, --replication-factor New replication factor for the topic [default: 1] - -h, --help Print help (see more with '--help') + -m, --max-topic-size + New max topic size in human-readable format like "unlimited" or "15GB" [default: server_default] + -r, --replication-factor + New replication factor for the topic [default: 1] + -h, --help + Print help (see more with '--help') "#, ), )) diff --git a/sdk/src/cli/topics/update_topic.rs b/sdk/src/cli/topics/update_topic.rs index 9a42ca5e1..f1393ff24 100644 --- a/sdk/src/cli/topics/update_topic.rs +++ b/sdk/src/cli/topics/update_topic.rs @@ -65,11 +65,13 @@ impl CliCommand for UpdateTopicCmd { })?; event!(target: PRINT_TARGET, Level::INFO, - "Topic with ID: {} updated name: {}, updated message expiry: {}, updated compression algorithm: {} in stream with ID: {}", + "Topic with ID: {} updated name: {}, updated message expiry: {}, updated compression algorithm: {}, updated max topic size: {}, updated replication factor: {} in stream with ID: {}", self.update_topic.topic_id, self.update_topic.name, self.message_expiry, self.update_topic.compression_algorithm, + self.max_topic_size, + self.replication_factor, self.update_topic.stream_id, ); diff --git a/sdk/src/utils/expiry.rs b/sdk/src/utils/expiry.rs index b21e40494..e86c5fb05 100644 --- a/sdk/src/utils/expiry.rs +++ b/sdk/src/utils/expiry.rs @@ -41,7 +41,7 @@ impl From<&IggyExpiry> for Option { impl Display for IggyExpiry { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::NeverExpire => write!(f, "none"), + Self::NeverExpire => write!(f, "never_expire"), Self::ServerDefault => write!(f, "server_default"), Self::ExpireDuration(value) => write!(f, "{value}"), } @@ -79,7 +79,7 @@ impl FromStr for IggyExpiry { fn from_str(s: &str) -> Result { let result = match s { - "unlimited" | "none" | "None" | "Unlimited" => IggyExpiry::NeverExpire, + "unlimited" | "none" | "None" | "Unlimited" | "never_expire" => IggyExpiry::NeverExpire, "default" | "server_default" | "Default" | "Server_default" => { IggyExpiry::ServerDefault } @@ -281,7 +281,7 @@ mod tests { #[test] fn should_check_display_expiry() { - assert_eq!(IggyExpiry::NeverExpire.to_string(), "none"); + assert_eq!(IggyExpiry::NeverExpire.to_string(), "never_expire"); assert_eq!( IggyExpiry::ExpireDuration(IggyDuration::from(333333000000)).to_string(), "3days 20h 35m 33s" diff --git a/server/src/streaming/topics/topic.rs b/server/src/streaming/topics/topic.rs index 794c5d5c0..666887067 100644 --- a/server/src/streaming/topics/topic.rs +++ b/server/src/streaming/topics/topic.rs @@ -248,7 +248,7 @@ impl fmt::Display for Topic { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { write!( f, - "Topic {{ id: {}, stream_id: {}, name: {}, path: {}, partitions: {}, message_expire: {}, max_topic_size: {}, replication_factor: {} }}", + "Topic {{ id: {}, stream_id: {}, name: {}, path: {}, partitions: {}, message_expiry: {}, max_topic_size: {}, replication_factor: {} }}", self.topic_id, self.stream_id, self.name,