fix(cli): update CLI and tests for server default parameters (#1545)

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.
apache · Feb 17, 2025 · 8c52d3f · 8c52d3f
1 parent 7a9bbde
commit 8c52d3f
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 58 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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 = ["[email protected]"]
 repository = "https://github.com/iggy-rs/iggy"

diff --git 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<IggyExpiry>,
 }
 
@@ -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<IggyExpiry>,
 }
 

diff --git 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>
           Topic ID to create
 
   -m, --max-topic-size <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>
           Replication factor for the topic
@@ -327,13 +329,17 @@ Arguments:
   <NAME>                   Name of the topic
   <PARTITIONS_COUNT>       Number of partitions inside the topic
   <COMPRESSION_ALGORITHM>  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>                      Topic ID to create
-  -m, --max-topic-size <MAX_TOPIC_SIZE>          Max topic size [default: unlimited]
-  -r, --replication-factor <REPLICATION_FACTOR>  Replication factor for the topic [default: 1]
-  -h, --help                                     Print help (see more with '--help')
+  -t, --topic-id <TOPIC_ID>
+          Topic ID to create
+  -m, --max-topic-size <MAX_TOPIC_SIZE>
+          Max topic size in human-readable format like "unlimited" or "15GB" [default: server_default]
+  -r, --replication-factor <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
@@ -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::<HumanDuration>().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 <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 <REPLICATION_FACTOR>
           New replication factor for the topic
@@ -435,12 +434,15 @@ Arguments:
   <TOPIC_ID>               Topic ID to update
   <NAME>                   New name for the topic
   <COMPRESSION_ALGORITHM>  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 <MAX_TOPIC_SIZE>          New max topic size [default: unlimited]
-  -r, --replication-factor <REPLICATION_FACTOR>  New replication factor for the topic [default: 1]
-  -h, --help                                     Print help (see more with '--help')
+  -m, --max-topic-size <MAX_TOPIC_SIZE>
+          New max topic size in human-readable format like "unlimited" or "15GB" [default: server_default]
+  -r, --replication-factor <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
@@ -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
@@ -41,7 +41,7 @@ impl From<&IggyExpiry> for Option<u64> {
 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<Self, Self::Err> {
         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
@@ -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,