Skip to content

Commit

Permalink
Move submitter code to separate module.
Browse files Browse the repository at this point in the history
  • Loading branch information
DFINITYManu committed Feb 19, 2025
1 parent 1fd0025 commit 503b7fb
Show file tree
Hide file tree
Showing 25 changed files with 107 additions and 97 deletions.
2 changes: 1 addition & 1 deletion rs/cli/src/commands/api_boundary_nodes/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use ic_types::PrincipalId;

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
ic_admin::{self},
};

Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/api_boundary_nodes/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use ic_types::PrincipalId;

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
ic_admin::{self},
};

Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/api_boundary_nodes/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use ic_types::PrincipalId;

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
ic_admin::{self},
};

Expand Down
3 changes: 2 additions & 1 deletion rs/cli/src/commands/firewall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ use tempfile::NamedTempFile;

use crate::{
ctx::DreContext,
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::ForumPostKind,
ic_admin::{IcAdminProposal, IcAdminProposalCommand, IcAdminProposalOptions},
proposal_executors::{ProducesProposalResult, ProposalResponseWithId, RunnableViaIcAdmin},
submitter::{SubmissionParameters, Submitter},
};

use super::{AuthRequirement, ExecutableCommand};
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/governance/propose/motion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use tokio::io::AsyncReadExt;

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
util::{extract_title_and_text, utf8},
};

Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/hostos/rollout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use ic_types::PrincipalId;

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
};

#[derive(Args, Debug)]
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/hostos/rollout_from_node_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use clap::{Args, ValueEnum};

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
operations::hostos_rollout::{NodeGroupUpdate, NumberOfNodes},
};

Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use itertools::Itertools;
use log::info;

use crate::{
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
ic_admin::IcAdminProposal,
};

Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/nodes/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use decentralization::subnets::NodesRemover;

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
};

#[derive(Args, Debug)]
Expand Down
3 changes: 2 additions & 1 deletion rs/cli/src/commands/propose.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use clap::{error::ErrorKind, Args};

use crate::{
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::ForumPostKind,
ic_admin::{IcAdminProposal, IcAdminProposalCommand},
submitter::{SubmissionParameters, Submitter},
};

use super::{AuthRequirement, ExecutableCommand};
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/subnet/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ic_types::PrincipalId;

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
};

#[derive(Args, Debug)]
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/subnet/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ic_types::PrincipalId;
use crate::{
auth::get_automation_neuron_default_path,
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
};

#[derive(Args, Debug)]
Expand Down
4 changes: 2 additions & 2 deletions rs/cli/src/commands/subnet/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ use clap::{error::ErrorKind, Args};
use ic_types::PrincipalId;
use itertools::Itertools;

use crate::forum::{ForumPostKind, SubmissionParameters};
use crate::forum::ForumPostKind;
use crate::submitter::{SubmissionParameters, Submitter};
use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::Submitter,
subnet_manager::SubnetTarget,
};

Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/subnet/rescue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ic_types::PrincipalId;

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
};

#[derive(Args, Debug)]
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/subnet/resize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ic_types::PrincipalId;

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
};

#[derive(Args, Debug)]
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/update_authorized_subnets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use itertools::Itertools;
use log::info;

use crate::{
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
ic_admin::{IcAdminProposal, IcAdminProposalCommand, IcAdminProposalOptions},
};

Expand Down
3 changes: 2 additions & 1 deletion rs/cli/src/commands/update_unassigned_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use clap::Args;
use ic_canisters::registry::RegistryCanisterWrapper;
use ic_types::PrincipalId;

use crate::forum::{ForumPostKind, SubmissionParameters, Submitter};
use crate::forum::ForumPostKind;
use crate::submitter::{SubmissionParameters, Submitter};

use super::{AuthRequirement, ExecutableCommand};

Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/version/revise/guest_os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use clap::{error::ErrorKind, Args};

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
};

#[derive(Debug, Args)]
Expand Down
2 changes: 1 addition & 1 deletion rs/cli/src/commands/version/revise/host_os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use clap::{error::ErrorKind, Args};

use crate::{
commands::{AuthRequirement, ExecutableCommand},
forum::{ForumPostKind, SubmissionParameters, Submitter},
forum::{ForumPostKind}, submitter::{SubmissionParameters, Submitter},
};

#[derive(Debug, Args)]
Expand Down
80 changes: 5 additions & 75 deletions rs/cli/src/forum/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,12 @@ use std::{path::PathBuf, str::FromStr};
use clap::Args as ClapArgs;
use futures::future::BoxFuture;
use ic_types::PrincipalId;
use log::warn;
use mockall::automock;

use crate::{
confirm::{ConfirmationModeOptions, HowToProceed},
proposal_executors::ProposalExecution,
util::yesno,
};

mod impls;

#[derive(Debug, Clone)]
pub enum ForumPostLinkVariant {
pub(crate) enum ForumPostLinkVariant {
Url(url::Url),
ManageOnDiscourse,
Ask,
Expand Down Expand Up @@ -120,15 +113,6 @@ impl ForumParameters {
}
}

#[derive(ClapArgs, Debug, Clone)]
pub struct SubmissionParameters {
#[clap(flatten)]
pub forum_parameters: ForumParameters,

#[clap(flatten)]
pub confirmation_mode: ConfirmationModeOptions,
}

// FIXME: this should become part of a new composite trait
// that builds on the ProducesProposalResults trait,
// so that we don't have to have a separate kind here, this just
Expand Down Expand Up @@ -158,7 +142,7 @@ pub trait ForumPost: Sync + Send {
}

#[derive(Clone)]
struct ForumContext {
pub struct ForumContext {
forum_opts: ForumParameters,
}

Expand All @@ -168,11 +152,6 @@ impl ForumContext {
Self { forum_opts }
}

// FIXME: turn into impl From.
fn from_opts(opts: &ForumParameters) -> Self {
Self::new(opts.clone())
}

pub fn client(&self) -> anyhow::Result<Box<dyn ForumPostHandler>> {
match &self.forum_opts.forum_post_link {
ForumPostLinkVariant::Url(u) => Ok(Box::new(impls::UserSuppliedLink { url: Some(u.clone()) }) as Box<dyn ForumPostHandler>),
Expand All @@ -183,57 +162,8 @@ impl ForumContext {
}
}

/// Helps the caller preview and then submit a proposal automatically,
/// handling the forum post part of the work as smoothly as possible.
pub struct Submitter {
mode: HowToProceed,
forum_parameters: ForumParameters,
}

impl From<&SubmissionParameters> for Submitter {
fn from(other: &SubmissionParameters) -> Self {
Self {
mode: (&other.confirmation_mode).into(),
forum_parameters: other.forum_parameters.clone(),
}
}
}

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<()> {
if let HowToProceed::Unconditional = self.mode {
} else {
execution.simulate().await?;
};

if let HowToProceed::Confirm = self.mode {
// Ask for confirmation
if !yesno("Do you want to continue?", false).await?? {
return Ok(());
}
}

if let HowToProceed::DryRun = self.mode {
Ok(())
} else {
let forum_post = ForumContext::from_opts(&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,
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)
}
}
}
impl From<&ForumParameters> for ForumContext {
fn from(p: &ForumParameters) -> Self {
Self::new(p.clone())
}
}
1 change: 1 addition & 0 deletions rs/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ mod proposal_executors;
mod qualification;
mod runner;
mod store;
mod submitter;
mod subnet_manager;
mod util;
1 change: 1 addition & 0 deletions rs/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ mod proposal_executors;
mod qualification;
mod runner;
mod store;
mod submitter;
mod subnet_manager;
#[cfg(test)]
mod unit_tests;
Expand Down
73 changes: 73 additions & 0 deletions rs/cli/src/submitter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
use clap::Args as ClapArgs;
use log::warn;

use crate::{
confirm::{ConfirmationModeOptions, HowToProceed},
forum::{ForumContext, ForumParameters, ForumPostKind},
proposal_executors::ProposalExecution,
util::yesno,
};

#[derive(ClapArgs, Debug, Clone)]
pub struct SubmissionParameters {
#[clap(flatten)]
pub forum_parameters: ForumParameters,

#[clap(flatten)]
pub confirmation_mode: ConfirmationModeOptions,
}

/// Helps the caller preview and then submit a proposal automatically,
/// handling the forum post part of the work as smoothly as possible.
pub struct Submitter {
mode: HowToProceed,
forum_parameters: ForumParameters,
}

impl From<&SubmissionParameters> for Submitter {
fn from(other: &SubmissionParameters) -> Self {
Self {
mode: (&other.confirmation_mode).into(),
forum_parameters: other.forum_parameters.clone(),
}
}
}

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<()> {
if let HowToProceed::Unconditional = self.mode {
} else {
execution.simulate().await?;
};

if let HowToProceed::Confirm = self.mode {
// Ask for confirmation
if !yesno("Do you want to continue?", false).await?? {
return Ok(());
}
}

if let HowToProceed::DryRun = self.mode {
Ok(())
} 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,
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)
}
}
}
}
}
3 changes: 2 additions & 1 deletion rs/cli/src/unit_tests/update_unassigned_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use crate::auth::Neuron;
use crate::commands::{update_unassigned_nodes::UpdateUnassignedNodes, ExecutableCommand};
use crate::confirm::ConfirmationModeOptions;
use crate::cordoned_feature_fetcher::MockCordonedFeatureFetcher;
use crate::forum::{ForumParameters, SubmissionParameters};
use crate::forum::ForumParameters;
use crate::ic_admin::MockIcAdmin;
use crate::submitter::SubmissionParameters;
use ic_management_backend::health::MockHealthStatusQuerier;
use ic_management_backend::{lazy_git::MockLazyGit, lazy_registry::MockLazyRegistry, proposal::MockProposalAgent};
use ic_management_types::{Network, Subnet};
Expand Down
Loading

0 comments on commit 503b7fb

Please sign in to comment.