Simplify `container network stop/logs`

[elizabethengelman]

trafficstars

With the next protocol release, handle the updates suggested in this discussion: https://github.com/stellar/stellar-cli/pull/1423#discussion_r1678472001

Some code to get started with:

diff --git a/cmd/soroban-cli/src/commands/network/container/logs.rs b/cmd/soroban-cli/src/commands/network/container/logs.rs
index c2151129..6c815883 100644
--- a/cmd/soroban-cli/src/commands/network/container/logs.rs
+++ b/cmd/soroban-cli/src/commands/network/container/logs.rs
@@ -1,8 +1,6 @@
 use futures_util::TryStreamExt;

-use crate::commands::network::container::shared::{
-    connect_to_docker, Error as ConnectionError, Network,
-};
+use crate::commands::network::container::shared::{connect_to_docker, Error as ConnectionError};

 use super::shared::Args;

@@ -20,14 +18,13 @@ pub struct Cmd {
     #[command(flatten)]
     pub container_args: Args,

-    /// Network container to tail (used in container name generation)
-    #[arg(required_unless_present = "container_name")]
-    pub network: Option<Network>,
+    /// Container to get logs from
+    pub name: String,
 }

 impl Cmd {
     pub async fn run(&self) -> Result<(), Error> {
-        let container_name = self.container_args.get_container_name(self.network);
+        let container_name = self.name.clone();
         let docker = connect_to_docker(&self.container_args.docker_host).await?;
         let logs_stream = &mut docker.logs(
             &container_name,
diff --git a/cmd/soroban-cli/src/commands/network/container/shared.rs b/cmd/soroban-cli/src/commands/network/container/shared.rs
index d2d0ccef..bf37dc7a 100644
--- a/cmd/soroban-cli/src/commands/network/container/shared.rs
+++ b/cmd/soroban-cli/src/commands/network/container/shared.rs
@@ -33,10 +33,6 @@ pub enum Error {

 #[derive(Debug, clap::Parser, Clone)]
 pub struct Args {
-    /// Optional argument to specify the container name
-    #[arg(short = 'c', long, required_unless_present = "network")]
-    pub container_name: Option<String>,
-
     /// Optional argument to override the default docker host. This is useful when you are using a non-standard docker host path for your Docker-compatible container runtime, e.g. Docker D
esktop defaults to $HOME/.docker/run/docker.sock instead of /var/run/docker.sock
     #[arg(short = 'd', long, help = DOCKER_HOST_HELP, env = "DOCKER_HOST")]
     pub docker_host: Option<String>,
@@ -49,33 +45,6 @@ impl Args {
             .map(|docker_host| format!("--docker-host {docker_host}"))
             .unwrap_or_default()
     }
-
-    pub(crate) fn get_container_name(&self, network: Option<Network>) -> String {
-        self.container_name.as_ref().map_or_else(
-            || {
-                format!(
-                    "stellar-{}",
-                    network.expect("Container name and/or network are required.")
-                )
-            },
-            |container_name| container_name.to_string(),
-        )
-    }
-
-    // This method is used in start.rs to create a message for the user to let them know how to stop the container they
-    // just started, and how to view its logs. For both `stop` and `logs` the user is able to pass in either the network
-    // (and we generate the container name) or the container name directly. Which is why we need to check if the
-    // container_name is present or not here.
-    pub(crate) fn get_container_name_arg(&self, network: Option<Network>) -> String {
-        self.container_name.as_ref().map_or_else(
-            || {
-                network
-                    .expect("Container name and/or network are required.")
-                    .to_string()
-            },
-            |container_name| format!("--container-name {container_name}"),
-        )
-    }
 }

 #[derive(ValueEnum, Debug, Copy, Clone, PartialEq)]
diff --git a/cmd/soroban-cli/src/commands/network/container/start.rs b/cmd/soroban-cli/src/commands/network/container/start.rs
index f9b5efdf..fcb0429d 100644
--- a/cmd/soroban-cli/src/commands/network/container/start.rs
+++ b/cmd/soroban-cli/src/commands/network/container/start.rs
@@ -33,6 +33,10 @@ pub struct Cmd {
     /// Network to start
     pub network: Network,

+    /// Optional argument to specify the container name
+    #[arg(long)]
+    pub name: Option<String>,
+
     /// Optional argument to specify the limits for the local network only
     #[arg(short = 'l', long)]
     pub limits: Option<String>,
@@ -89,7 +93,7 @@ async fn run_docker_command(cmd: &Cmd) -> Result<(), Error> {
         ..Default::default()
     };

-    let container_name = cmd.container_args.get_container_name(Some(cmd.network));
+    let container_name = get_container_name(cmd);
     let create_container_response = docker
         .create_container(
             Some(CreateContainerOptions {
@@ -112,10 +116,17 @@ async fn run_docker_command(cmd: &Cmd) -> Result<(), Error> {
     Ok(())
 }

+fn get_container_name(cmd: &Cmd) -> String {
+    cmd.name.as_ref().map_or_else(
+        || format!("stellar-{}", cmd.network),
+        std::string::ToString::to_string,
+    )
+}
+
 fn print_log_message(cmd: &Cmd) {
     let log_message = format!(
-        "ℹ️ To see the logs for this container run: stellar network container logs {arg} {additional_flags}",
-        arg = cmd.container_args.get_container_name_arg(Some(cmd.network)),
+        "ℹ️ To see the logs for this container run: stellar network container logs {name} {additional_flags}",
+    )
+}
+
 fn print_log_message(cmd: &Cmd) {
     let log_message = format!(
-        "ℹ️ To see the logs for this container run: stellar network container logs {arg} {additional_flags}",
-        arg = cmd.container_args.get_container_name_arg(Some(cmd.network)),
+        "ℹ️ To see the logs for this container run: stellar network container logs {name} {additional_flags}",
+        name = get_container_name(cmd),
         additional_flags = cmd.container_args.get_additional_flags(),
     );
     println!("{log_message}");
@@ -123,8 +134,8 @@ fn print_log_message(cmd: &Cmd) {

 fn print_stop_message(cmd: &Cmd) {
     let stop_message = format!(
-        "ℹ️ To stop this container run: stellar network container stop {arg} {additional_flags}",
-        arg = cmd.container_args.get_container_name_arg(Some(cmd.network)),
+        "ℹ️ To stop this container run: stellar network container stop {name} {additional_flags}",
+        name = get_container_name(cmd),
         additional_flags = cmd.container_args.get_additional_flags(),
     );
     println!("{stop_message}");
diff --git a/cmd/soroban-cli/src/commands/network/container/stop.rs b/cmd/soroban-cli/src/commands/network/container/stop.rs
index 596fcab4..63b7e6ac 100644
--- a/cmd/soroban-cli/src/commands/network/container/stop.rs
+++ b/cmd/soroban-cli/src/commands/network/container/stop.rs
@@ -1,5 +1,5 @@
 use crate::commands::network::container::shared::{
-    connect_to_docker, Error as BollardConnectionError, Network,
+    connect_to_docker, Error as BollardConnectionError,
 };

 use super::shared::Args;
@@ -25,14 +25,13 @@ pub struct Cmd {
     #[command(flatten)]
     pub container_args: Args,

-    /// Network to stop (used in container name generation)
-    #[arg(required_unless_present = "container_name")]
-    pub network: Option<Network>,
+    /// Container to stop
+    pub name: String,
 }

 impl Cmd {
     pub async fn run(&self) -> Result<(), Error> {
-        let container_name = self.container_args.get_container_name(self.network);
+        let container_name = self.name.clone();
         let docker = connect_to_docker(&self.container_args.docker_host).await?;
         println!("ℹ️ Stopping container: {container_name}");
         docker

[janewang]

@elizabethengelman Can this issue be closed? I see the PR merged