Cleanly handle propose_run().

dfinity · Jan 31, 2025 · f27624a · f27624a
1 parent 33610a5
commit f27624a
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 21 deletions.
diff --git a/rs/cli/src/commands/update_authorized_subnets.rs b/rs/cli/src/commands/update_authorized_subnets.rs
@@ -156,13 +156,8 @@ impl ExecutableCommand for UpdateAuthorizedSubnets {
             forum_post_link: Some("[comment]: <> (Link will be added on actual execution)".to_string()),
         };
 
-        // Reviewers / @nikola: why do we do this here?  Everywhere else we just propose_run.
-        if !ic_admin.propose_print_and_confirm(cmd.clone(), opts.clone()).await? {
-            return Ok(());
-        }
-
         forum_enabled_proposer(&self.forum_parameters, &ctx, ic_admin)
-            .propose_submit(cmd, opts, ForumPostKind::AuthorizedSubnetsUpdate { body: summary })
+            .propose_run(cmd, opts, ForumPostKind::AuthorizedSubnetsUpdate { body: summary })
             .await
     }
 }

diff --git a/rs/cli/src/forum/ic_admin.rs b/rs/cli/src/forum/ic_admin.rs
@@ -1,6 +1,6 @@
 use std::sync::Arc;
 
-use log::info;
+use log::{info, warn};
 
 use super::{ForumParameters, ForumPostKind};
 use crate::{
@@ -24,24 +24,55 @@ pub fn forum_enabled_proposer(forum_parameters: &ForumParameters, ctx: &DreConte
 
 impl IcAdminProxy {
     async fn propose_or_submit(&self, cmd: ProposeCommand, opts: ProposeOptions, kind: ForumPostKind, directly_submit: bool) -> anyhow::Result<()> {
+        if !directly_submit
+            && !self
+                .ic_admin
+                .propose_print_and_confirm(
+                    cmd.clone(),
+                    ProposeOptions {
+                        forum_post_link: Some("<forum post URL will be supplied once you confirm>".into()),
+                        ..opts.clone()
+                    },
+                )
+                .await?
+        {
+            return Ok(());
+        };
+
         let forum_post = super::ForumContext::from_opts(&self.forum_parameters, self.simulate)
             .client()?
             .forum_post(kind)
             .await?;
-        let opts = ProposeOptions {
-            forum_post_link: forum_post.url().map(|s| s.into()),
-            ..opts
-        };
-        let res = if directly_submit {
-            self.ic_admin.propose_submit(cmd, opts).await?
-        } else {
-            self.ic_admin.propose_run(cmd, opts).await?
-        };
-        if self.simulate {
-            info!("Simulating that the proposal returned by ic-admin is 123456");
-            forum_post.add_proposal_url(123456).await
-        } else {
-            forum_post.update_by_parsing_ic_admin_response(res).await
+        match self
+            .ic_admin
+            .propose_submit(
+                cmd,
+                ProposeOptions {
+                    forum_post_link: forum_post.url().map(|s| s.into()),
+                    ..opts
+                },
+            )
+            .await
+        {
+            Ok(res) => {
+                if self.simulate {
+                    info!("Simulating that the proposal returned by ic-admin is 123456");
+                    forum_post.add_proposal_url(123456).await
+                } else {
+                    forum_post.update_by_parsing_ic_admin_response(res).await
+                }
+            }
+            Err(e) => {
+                if let Some(forum_post_url) = forum_post.url() {
+                    // Here we would ask the forum post code to delete the post since
+                    // the submission has failed... that is, if we had that feature.
+                    warn!(
+                        "Forum post {} may have been created for this proposal, but proposal submission failed.  Please delete the forum post if necessary, as it now serves no purpose.",
+                        forum_post_url
+                    );
+                };
+                Err(e)
+            }
         }
     }
     pub async fn propose_run(&self, cmd: ProposeCommand, opts: ProposeOptions, kind: ForumPostKind) -> anyhow::Result<()> {