From 0674cc56f6d25c22434e074ca210bab46d59e3e8 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 26 Aug 2024 09:41:32 -0700 Subject: [PATCH 1/4] omdb nexus blueprints target set --diff` --- dev-tools/omdb/src/bin/omdb/nexus.rs | 47 ++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index ede2743404..08d1ee6e07 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -244,6 +244,10 @@ struct BlueprintTargetSetArgs { blueprint_id: Uuid, /// whether this blueprint should be enabled enabled: BlueprintTargetSetEnabled, + /// if specified, diff against the current target and wait for confirmation + /// before proceeding + #[clap(long)] + diff: bool, } #[derive(Debug, Clone, Copy, ValueEnum)] @@ -1734,12 +1738,43 @@ async fn cmd_nexus_blueprints_target_set( // operator. (In the case of the current target blueprint being changed // entirely, that will result in a failure to set the current target // below, because its parent will no longer be the current target.) - BlueprintTargetSetEnabled::Inherit => client - .blueprint_target_view() - .await - .map(|current| current.into_inner().enabled) - .context("failed to fetch current target blueprint")?, + BlueprintTargetSetEnabled::Inherit => { + client + .blueprint_target_view() + .await + .context("failed to fetch current target blueprint")? + .into_inner() + .enabled + } }; + + if args.diff { + let blueprint1 = client + .blueprint_view( + &client + .blueprint_target_view() + .await + .context("failed to fetch current target blueprint")? + .into_inner() + .target_id, + ) + .await + .context("failed to fetch target blueprint")? + .into_inner(); + let blueprint2 = + client.blueprint_view(&args.blueprint_id).await.with_context( + || format!("fetching blueprint {}", args.blueprint_id), + )?; + let diff = blueprint2.diff_since_blueprint(&blueprint1); + println!("{}", diff.display()); + println!( + "\nDo you want to make {} the target blueprint?", + args.blueprint_id + ); + let mut prompt = ConfirmationPrompt::new(); + prompt.read_and_validate("y/N", "y")?; + } + client .blueprint_target_set(&nexus_client::types::BlueprintTargetSet { target_id: args.blueprint_id, @@ -1966,7 +2001,7 @@ impl ConfirmationPrompt { { Ok(input) } else { - bail!("expungement aborted") + bail!("operation aborted") } } From 201f3032800400165b9084712d5483f9b2cd5472 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 26 Aug 2024 09:52:48 -0700 Subject: [PATCH 2/4] just fetch the target once --- dev-tools/omdb/src/bin/omdb/nexus.rs | 72 ++++++++++++++-------------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 08d1ee6e07..2cbac2d5b2 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -1726,6 +1726,40 @@ async fn cmd_nexus_blueprints_target_set( args: &BlueprintTargetSetArgs, _destruction_token: DestructiveOperationToken, ) -> Result<(), anyhow::Error> { + let found_enabled = if args.diff + || matches!(args.enabled, BlueprintTargetSetEnabled::Inherit) + { + let current_target = client + .blueprint_target_view() + .await + .context("failed to fetch current target blueprint")? + .into_inner(); + + if args.diff { + let blueprint1 = client + .blueprint_view(¤t_target.target_id) + .await + .context("failed to fetch target blueprint")? + .into_inner(); + let blueprint2 = + client.blueprint_view(&args.blueprint_id).await.with_context( + || format!("fetching blueprint {}", args.blueprint_id), + )?; + let diff = blueprint2.diff_since_blueprint(&blueprint1); + println!("{}", diff.display()); + println!( + "\nDo you want to make {} the target blueprint?", + args.blueprint_id + ); + let mut prompt = ConfirmationPrompt::new(); + prompt.read_and_validate("y/N", "y")?; + } + + Some(current_target.enabled) + } else { + None + }; + let enabled = match args.enabled { BlueprintTargetSetEnabled::Enabled => true, BlueprintTargetSetEnabled::Disabled => false, @@ -1738,43 +1772,11 @@ async fn cmd_nexus_blueprints_target_set( // operator. (In the case of the current target blueprint being changed // entirely, that will result in a failure to set the current target // below, because its parent will no longer be the current target.) - BlueprintTargetSetEnabled::Inherit => { - client - .blueprint_target_view() - .await - .context("failed to fetch current target blueprint")? - .into_inner() - .enabled - } + // + // unwrap(): this is always `Some` due to the branch above. + BlueprintTargetSetEnabled::Inherit => found_enabled.unwrap(), }; - if args.diff { - let blueprint1 = client - .blueprint_view( - &client - .blueprint_target_view() - .await - .context("failed to fetch current target blueprint")? - .into_inner() - .target_id, - ) - .await - .context("failed to fetch target blueprint")? - .into_inner(); - let blueprint2 = - client.blueprint_view(&args.blueprint_id).await.with_context( - || format!("fetching blueprint {}", args.blueprint_id), - )?; - let diff = blueprint2.diff_since_blueprint(&blueprint1); - println!("{}", diff.display()); - println!( - "\nDo you want to make {} the target blueprint?", - args.blueprint_id - ); - let mut prompt = ConfirmationPrompt::new(); - prompt.read_and_validate("y/N", "y")?; - } - client .blueprint_target_set(&nexus_client::types::BlueprintTargetSet { target_id: args.blueprint_id, From f9beb4c570c75d672e5b6a02e86f753245032fd2 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 26 Aug 2024 11:08:17 -0700 Subject: [PATCH 3/4] clean up use of target blueprint value --- dev-tools/omdb/src/bin/omdb/nexus.rs | 65 ++++++++++++++-------------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 2cbac2d5b2..70ce2db392 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -51,6 +51,7 @@ use std::collections::BTreeMap; use std::collections::BTreeSet; use std::str::FromStr; use tabled::Tabled; +use tokio::sync::OnceCell; use uuid::Uuid; /// Arguments to the "omdb nexus" subcommand @@ -1726,40 +1727,38 @@ async fn cmd_nexus_blueprints_target_set( args: &BlueprintTargetSetArgs, _destruction_token: DestructiveOperationToken, ) -> Result<(), anyhow::Error> { - let found_enabled = if args.diff - || matches!(args.enabled, BlueprintTargetSetEnabled::Inherit) - { - let current_target = client - .blueprint_target_view() + // Helper to only fetch the current target once. We may need it immediately + // if `args.diff` is true, or later if `args.enabled` is "inherit" (or + // both). + let current_target = OnceCell::new(); + let get_current_target = || async { + current_target + .get_or_try_init(|| client.blueprint_target_view()) .await - .context("failed to fetch current target blueprint")? - .into_inner(); - - if args.diff { - let blueprint1 = client - .blueprint_view(¤t_target.target_id) - .await - .context("failed to fetch target blueprint")? - .into_inner(); - let blueprint2 = - client.blueprint_view(&args.blueprint_id).await.with_context( - || format!("fetching blueprint {}", args.blueprint_id), - )?; - let diff = blueprint2.diff_since_blueprint(&blueprint1); - println!("{}", diff.display()); - println!( - "\nDo you want to make {} the target blueprint?", - args.blueprint_id - ); - let mut prompt = ConfirmationPrompt::new(); - prompt.read_and_validate("y/N", "y")?; - } - - Some(current_target.enabled) - } else { - None + .context("failed to fetch current target blueprint") }; + if args.diff { + let current_target = get_current_target().await?; + let blueprint1 = client + .blueprint_view(¤t_target.target_id) + .await + .context("failed to fetch target blueprint")? + .into_inner(); + let blueprint2 = + client.blueprint_view(&args.blueprint_id).await.with_context( + || format!("fetching blueprint {}", args.blueprint_id), + )?; + let diff = blueprint2.diff_since_blueprint(&blueprint1); + println!("{}", diff.display()); + println!( + "\nDo you want to make {} the target blueprint?", + args.blueprint_id + ); + let mut prompt = ConfirmationPrompt::new(); + prompt.read_and_validate("y/N", "y")?; + } + let enabled = match args.enabled { BlueprintTargetSetEnabled::Enabled => true, BlueprintTargetSetEnabled::Disabled => false, @@ -1774,7 +1773,9 @@ async fn cmd_nexus_blueprints_target_set( // below, because its parent will no longer be the current target.) // // unwrap(): this is always `Some` due to the branch above. - BlueprintTargetSetEnabled::Inherit => found_enabled.unwrap(), + BlueprintTargetSetEnabled::Inherit => { + get_current_target().await?.enabled + } }; client From b8c748ff6feea175f75857325381972fc97cea10 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 26 Aug 2024 11:16:29 -0700 Subject: [PATCH 4/4] remove spurious comment --- dev-tools/omdb/src/bin/omdb/nexus.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 70ce2db392..db9e2cba52 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -1771,8 +1771,6 @@ async fn cmd_nexus_blueprints_target_set( // operator. (In the case of the current target blueprint being changed // entirely, that will result in a failure to set the current target // below, because its parent will no longer be the current target.) - // - // unwrap(): this is always `Some` due to the branch above. BlueprintTargetSetEnabled::Inherit => { get_current_target().await?.enabled }