Print proposal ID when a proposal is issued. (#1323)

DFINITYManu · web-flow · commit 481bd246ce0b · 2025-02-28T15:47:14.000+01:00
diff --git a/rs/cli/src/commands/api_boundary_nodes/add.rs b/rs/cli/src/commands/api_boundary_nodes/add.rs
@@ -35,7 +35,7 @@ impl ExecutableCommand for Add {
 
     async fn execute(&self, ctx: crate::ctx::DreContext) -> anyhow::Result<()> {
         Submitter::from(&self.submission_parameters)
-            .propose(
+            .propose_and_print(
                 ctx.ic_admin_executor().await?.execution(ic_admin::IcAdminProposal::new(
                     ic_admin::IcAdminProposalCommand::AddApiBoundaryNodes {
                         nodes: self.nodes.to_vec(),
diff --git a/rs/cli/src/commands/api_boundary_nodes/remove.rs b/rs/cli/src/commands/api_boundary_nodes/remove.rs
@@ -31,7 +31,7 @@ impl ExecutableCommand for Remove {
 
     async fn execute(&self, ctx: crate::ctx::DreContext) -> anyhow::Result<()> {
         Submitter::from(&self.submission_parameters)
-            .propose(
+            .propose_and_print(
                 ctx.ic_admin_executor().await?.execution(ic_admin::IcAdminProposal::new(
                     ic_admin::IcAdminProposalCommand::RemoveApiBoundaryNodes { nodes: self.nodes.to_vec() },
                     ic_admin::IcAdminProposalOptions {
diff --git a/rs/cli/src/commands/api_boundary_nodes/update.rs b/rs/cli/src/commands/api_boundary_nodes/update.rs
@@ -34,7 +34,7 @@ impl ExecutableCommand for Update {
 
     async fn execute(&self, ctx: crate::ctx::DreContext) -> anyhow::Result<()> {
         Submitter::from(&self.submission_parameters)
-            .propose(
+            .propose_and_print(
                 ctx.ic_admin_executor().await?.execution(ic_admin::IcAdminProposal::new(
                     ic_admin::IcAdminProposalCommand::DeployGuestosToSomeApiBoundaryNodes {
                         nodes: self.nodes.to_vec(),
diff --git a/rs/cli/src/commands/firewall.rs b/rs/cli/src/commands/firewall.rs
@@ -235,8 +235,12 @@ impl Firewall {
         let hash = parsed.hash;
         info!("Computed hash for firewall rule at position '{}': {}", positions, hash);
 
+        // We should probably not be calling the print variant here, since
+        // this is called in a loop by the caller of create_proposal.  Perhaps
+        // we should summarize the proposals that were submitted, or the errors
+        // that were returned.
         Submitter::from(submission_parameters)
-            .propose(
+            .propose_and_print(
                 ctx.ic_admin_executor().await?.execution(FirewallModifyCommand {
                     test_command,
                     hash,
diff --git a/rs/cli/src/commands/governance/propose/motion.rs b/rs/cli/src/commands/governance/propose/motion.rs
@@ -81,7 +81,7 @@ impl ExecutableCommand for Motion {
         };
 
         Submitter::from(&self.submission_parameters)
-            .propose(
+            .propose_and_print(
                 ctx.governance_executor().await?.execution(request),
                 ForumPostKind::Motion {
                     title: title.clone(),
diff --git a/rs/cli/src/commands/hostos/rollout.rs b/rs/cli/src/commands/hostos/rollout.rs
@@ -31,7 +31,7 @@ impl ExecutableCommand for Rollout {
     async fn execute(&self, ctx: crate::ctx::DreContext) -> anyhow::Result<()> {
         let runner_proposal = ctx.runner().await?.hostos_rollout(self.nodes.clone(), &self.version, None)?;
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
             .await
     }
 
diff --git a/rs/cli/src/commands/hostos/rollout_from_node_group.rs b/rs/cli/src/commands/hostos/rollout_from_node_group.rs
@@ -101,7 +101,7 @@ impl ExecutableCommand for RolloutFromNodeGroup {
 
         let runner_proposal = runner.hostos_rollout(nodes_to_update, &self.version, Some(summary))?;
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
             .await
     }
 
diff --git a/rs/cli/src/commands/network.rs b/rs/cli/src/commands/network.rs
@@ -177,6 +177,8 @@ impl ExecutableCommand for Network {
             return Ok(());
         }
 
+        let mut submitted_proposals = vec![];
+
         for proposal in proposals {
             if let crate::ic_admin::IcAdminProposalCommand::ChangeSubnetMembership { subnet_id, .. } = &proposal.command {
                 let body = match (&proposal.options.motivation, &proposal.options.summary) {
@@ -191,17 +193,20 @@ impl ExecutableCommand for Network {
                         continue;
                     }
                 };
-                if let Err(e) = Submitter::from(&self.submission_parameters)
+                match Submitter::from(&self.submission_parameters)
                     .propose(
                         ctx.ic_admin_executor().await?.execution(proposal.clone()),
                         ForumPostKind::ReplaceNodes { subnet_id: *subnet_id, body },
                     )
                     .await
                 {
-                    errors.push(DetailedError {
-                        proposal: Some(proposal),
-                        error: e,
-                    });
+                    Err(e) => {
+                        errors.push(DetailedError {
+                            proposal: Some(proposal),
+                            error: e,
+                        });
+                    }
+                    Ok(p) => submitted_proposals.push(p),
                 }
             } else {
                 errors.push(DetailedError {
@@ -211,6 +216,10 @@ impl ExecutableCommand for Network {
             }
         }
 
+        for p in submitted_proposals.iter().flatten() {
+            println! {"{}", p};
+        }
+
         if errors.is_empty() {
             Ok(())
         } else {
diff --git a/rs/cli/src/commands/nodes/remove.rs b/rs/cli/src/commands/nodes/remove.rs
@@ -52,7 +52,7 @@ impl ExecutableCommand for Remove {
             })
             .await?;
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
             .await
     }
 
diff --git a/rs/cli/src/commands/propose.rs b/rs/cli/src/commands/propose.rs
@@ -44,7 +44,7 @@ impl ExecutableCommand for Propose {
         let cmd = IcAdminProposal::new(IcAdminProposalCommand::Raw(args.clone()), Default::default());
 
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(cmd), ForumPostKind::Generic)
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(cmd), ForumPostKind::Generic)
             .await
     }
 
diff --git a/rs/cli/src/commands/subnet/create.rs b/rs/cli/src/commands/subnet/create.rs
@@ -87,7 +87,7 @@ impl ExecutableCommand for Create {
             None => return Ok(()),
         };
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic) // FIXME once the Proposable struct gains knowledge of how to create a forum post, then it won't be necessary to pass two different structs.
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic) // FIXME once the Proposable struct gains knowledge of how to create a forum post, then it won't be necessary to pass two different structs.
             .await
     }
 
diff --git a/rs/cli/src/commands/subnet/deploy.rs b/rs/cli/src/commands/subnet/deploy.rs
@@ -34,7 +34,7 @@ impl ExecutableCommand for Deploy {
     async fn execute(&self, ctx: crate::ctx::DreContext) -> anyhow::Result<()> {
         let runner_proposal = ctx.runner().await?.deploy(&self.id, &self.version).await?;
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
             .await
     }
 
diff --git a/rs/cli/src/commands/subnet/replace.rs b/rs/cli/src/commands/subnet/replace.rs
@@ -79,7 +79,7 @@ impl ExecutableCommand for Replace {
         };
 
         Submitter::from(&self.submission_parameters)
-            .propose(
+            .propose_and_print(
                 ctx.ic_admin_executor().await?.execution(runner_proposal.clone()),
                 match subnet_change_response.subnet_id {
                     Some(id) => ForumPostKind::ReplaceNodes {
diff --git a/rs/cli/src/commands/subnet/rescue.rs b/rs/cli/src/commands/subnet/rescue.rs
@@ -35,7 +35,7 @@ impl ExecutableCommand for Rescue {
             None => return Ok(()),
         };
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
             .await
     }
 
diff --git a/rs/cli/src/commands/subnet/resize.rs b/rs/cli/src/commands/subnet/resize.rs
@@ -76,7 +76,7 @@ impl ExecutableCommand for Resize {
             None => return Ok(()),
         };
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
             .await
     }
 
diff --git a/rs/cli/src/commands/update_authorized_subnets.rs b/rs/cli/src/commands/update_authorized_subnets.rs
@@ -157,7 +157,7 @@ impl ExecutableCommand for UpdateAuthorizedSubnets {
         );
 
         Submitter::from(&self.submission_parameters)
-            .propose(
+            .propose_and_print(
                 ctx.ic_admin_executor().await?.execution(prop),
                 ForumPostKind::AuthorizedSubnetsUpdate { body: summary },
             )
diff --git a/rs/cli/src/commands/update_unassigned_nodes.rs b/rs/cli/src/commands/update_unassigned_nodes.rs
@@ -49,7 +49,7 @@ impl ExecutableCommand for UpdateUnassignedNodes {
             None => return Ok(()),
         };
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
             .await
     }
 
diff --git a/rs/cli/src/commands/version/revise/guest_os.rs b/rs/cli/src/commands/version/revise/guest_os.rs
@@ -48,7 +48,7 @@ impl ExecutableCommand for GuestOs {
             )
             .await?;
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
             .await
     }
 
diff --git a/rs/cli/src/commands/version/revise/host_os.rs b/rs/cli/src/commands/version/revise/host_os.rs
@@ -48,7 +48,7 @@ impl ExecutableCommand for HostOs {
             )
             .await?;
         Submitter::from(&self.submission_parameters)
-            .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
+            .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)
             .await
     }
 
diff --git a/rs/cli/src/proposal_executors.rs b/rs/cli/src/proposal_executors.rs
@@ -1,4 +1,4 @@
-use std::fmt::Debug;
+use std::fmt::{self, Debug, Display};
 
 use futures::future::BoxFuture;
 use ic_nns_governance_api::pb::v1::{manage_neuron_response::MakeProposalResponse, MakeProposalRequest};
@@ -9,6 +9,7 @@ use url::Url;
 /// of the kind of responses that contain a proposal ID.
 /// Not every proposal response from ic-admin can be deserialized
 /// into a proposal ID, but most of them can.
+#[derive(Clone)]
 pub struct ProposalResponseWithId(u64);
 
 impl From<ProposalResponseWithId> for u64 {
@@ -24,6 +25,12 @@ impl TryFrom<u64> for ProposalResponseWithId {
     }
 }
 
+impl Display for ProposalResponseWithId {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "proposal {}", self.0)
+    }
+}
+
 impl TryFrom<String> for ProposalResponseWithId {
     type Error = anyhow::Error;
 
diff --git a/rs/cli/src/submitter.rs b/rs/cli/src/submitter.rs
@@ -4,7 +4,7 @@ use log::warn;
 use crate::{
     confirm::{ConfirmationModeOptions, HowToProceed},
     forum::{ForumContext, ForumParameters, ForumPostKind},
-    proposal_executors::ProposalExecution,
+    proposal_executors::{ProposalExecution, ProposalResponseWithId},
     util::yesno,
 };
 
@@ -36,7 +36,11 @@ impl From<&SubmissionParameters> for Submitter {
 impl Submitter {
     /// Submits a proposal (maybe in dry-run mode) with confirmation from the user, unless the user
     /// specifies in the command line that he wants no confirmation (--yes).
-    pub async fn propose(&self, execution: Box<dyn ProposalExecution>, kind: ForumPostKind) -> anyhow::Result<()> {
+    ///
+    /// When a proposal has successfully been executed (as opposed to just simulated),
+    /// the result will contain a Some(ProposalResponseWithId).
+    #[must_use = "Are you implementing a CLI command?  Did you forget to print the proposal ID possibly returned by this function?  Consider propose_and_print instead."]
+    pub async fn propose(&self, execution: Box<dyn ProposalExecution>, kind: ForumPostKind) -> anyhow::Result<Option<ProposalResponseWithId>> {
         if let HowToProceed::Unconditional = self.mode {
         } else {
             execution.simulate().await?;
@@ -45,17 +49,29 @@ impl Submitter {
         if let HowToProceed::Confirm = self.mode {
             // Ask for confirmation
             if !yesno("Do you want to continue?", false).await?? {
-                return Ok(());
+                return Ok(None);
             }
         }
 
         if let HowToProceed::DryRun = self.mode {
-            Ok(())
+            Ok(None)
         } else {
             let forum_post = ForumContext::from(&self.forum_parameters).client()?.forum_post(kind).await?;
             let res = execution.submit(forum_post.url()).await;
             match res {
-                Ok(res) => forum_post.add_proposal_url(res.into()).await,
+                Ok(res) => {
+                    match forum_post.add_proposal_url(res.clone().into()).await {
+                        Ok(_) => (),
+                        Err(e) => {
+                            if let Some(forum_post_url) = forum_post.url() {
+                                warn!("Failed to add the proposal URL to forum post {}: {}", forum_post_url, e)
+                            } else {
+                                warn!("Failed to add the proposal URL to forum post: {}", e)
+                            }
+                        }
+                    };
+                    Ok(Some(res))
+                }
                 Err(e) => {
                     if let Some(forum_post_url) = forum_post.url() {
                         // Here we would ask the forum post code to delete the post since
@@ -70,4 +86,19 @@ impl Submitter {
             }
         }
     }
+
+    /// Submits a proposal (maybe in dry-run mode) with confirmation from the user, by calling
+    /// Self.propose().
+    ///
+    /// When a proposal has successfully been executed (as opposed to just simulated),
+    /// the proposal ID will be printed to standard output as "proposal XXXXXX".
+    ///
+    /// You should only call this convenience method if all your code is going to do is print
+    /// the returned proposal ID and exit.
+    pub async fn propose_and_print(&self, execution: Box<dyn ProposalExecution>, kind: ForumPostKind) -> anyhow::Result<()> {
+        if let Some(p) = self.propose(execution, kind).await? {
+            println!("{}", p)
+        };
+        Ok(())
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ impl ExecutableCommand for Rollout {`
`31`	`31`	`async fn execute(&self, ctx: crate::ctx::DreContext) -> anyhow::Result<()> {`
`32`	`32`	`let runner_proposal = ctx.runner().await?.hostos_rollout(self.nodes.clone(), &self.version, None)?;`
`33`	`33`	`Submitter::from(&self.submission_parameters)`
`34`		`- .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)`
	`34`	`+ .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)`
`35`	`35`	`.await`
`36`	`36`	`}`
`37`	`37`
Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ impl ExecutableCommand for RolloutFromNodeGroup {`
`101`	`101`
`102`	`102`	`let runner_proposal = runner.hostos_rollout(nodes_to_update, &self.version, Some(summary))?;`
`103`	`103`	`Submitter::from(&self.submission_parameters)`
`104`		`- .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)`
	`104`	`+ .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)`
`105`	`105`	`.await`
`106`	`106`	`}`
`107`	`107`
Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ impl ExecutableCommand for Remove {`
`52`	`52`	`})`
`53`	`53`	`.await?;`
`54`	`54`	`Submitter::from(&self.submission_parameters)`
`55`		`- .propose(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)`
	`55`	`+ .propose_and_print(ctx.ic_admin_executor().await?.execution(runner_proposal), ForumPostKind::Generic)`
`56`	`56`	`.await`
`57`	`57`	`}`
`58`	`58`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ impl ExecutableCommand for Propose {`
`44`	`44`	`let cmd = IcAdminProposal::new(IcAdminProposalCommand::Raw(args.clone()), Default::default());`
`45`	`45`
`46`	`46`	`Submitter::from(&self.submission_parameters)`
`47`		`- .propose(ctx.ic_admin_executor().await?.execution(cmd), ForumPostKind::Generic)`
	`47`	`+ .propose_and_print(ctx.ic_admin_executor().await?.execution(cmd), ForumPostKind::Generic)`
`48`	`48`	`.await`
`49`	`49`	`}`
`50`	`50`