Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update dfx deploy output #4104

Merged
merged 10 commits into from
Feb 11, 2025
4 changes: 2 additions & 2 deletions e2e/tests-dfx/deploy.bash
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ teardown() {
(
cd src
assert_command dfx deploy
assert_match "Installing code for"
assert_match "Installed code for"
)

assert_command dfx canister call hello_backend greet '("Banzai")'
assert_eq '("Hello, Banzai!")'

assert_command dfx deploy
assert_not_match "Installing code for"
assert_not_match "Installed code for"
assert_match "is already installed"
}

Expand Down
4 changes: 2 additions & 2 deletions e2e/tests-dfx/install.bash
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ teardown() {

assert_command dfx canister install --all

assert_match "Installing code for canister e2e_project_backend"
assert_match "Installed code for canister e2e_project_backend"
}

@test "install succeeds with network name" {
Expand All @@ -52,7 +52,7 @@ teardown() {

assert_command dfx canister install --all --network local

assert_match "Installing code for canister e2e_project_backend"
assert_match "Installed code for canister e2e_project_backend"
}

@test "install fails with network name that is not in dfx.json" {
Expand Down
18 changes: 9 additions & 9 deletions e2e/tests-dfx/mode_reinstall.bash
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ teardown() {
assert_command dfx canister install --mode=reinstall hello_backend

assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER"
assert_match "Reinstalling code for canister hello_backend"
assert_match "Reinstalled code for canister hello_backend"
)
}

Expand All @@ -54,7 +54,7 @@ teardown() {

assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER"

assert_not_match "Installing code for canister"
assert_not_match "Installed code for canister"
assert_contains "Refusing to install canister without approval"
assert_contains "User declined consent"
)
Expand All @@ -77,7 +77,7 @@ teardown() {
assert_command dfx deploy --mode=reinstall hello_backend

assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER"
assert_match "Reinstalling code for canister hello_backend"
assert_match "Reinstalled code for canister hello_backend"
)
}

Expand All @@ -90,7 +90,7 @@ teardown() {

assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER"

assert_not_match "Installing code for canister"
assert_not_match "Installed code for canister"
assert_contains "Refusing to install canister without approval"
assert_contains "User declined consent"
)
Expand Down Expand Up @@ -123,7 +123,7 @@ teardown() {
assert_match "You are about to reinstall the hello_frontend canister."
assert_not_match "You are about to reinstall the hello_backend canister."
assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER"
assert_match "Reinstalling code for canister hello_frontend,"
assert_match "Reinstalled code for canister hello_frontend,"
)

# the hello_backend canister should not have been upgraded (which would reset the non-stable var)
Expand All @@ -141,24 +141,24 @@ teardown() {
assert_command dfx deploy --mode=reinstall hello_backend

assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER"
assert_match "Reinstalling code for canister hello_backend"
assert_match "Reinstalled code for canister hello_backend"
)
echo y | (
assert_command dfx deploy --mode=reinstall hello_backend

assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER"
assert_match "Reinstalling code for canister hello_backend"
assert_match "Reinstalled code for canister hello_backend"
)
echo YES | (
assert_command dfx deploy --mode=reinstall hello_backend

assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER"
assert_match "Reinstalling code for canister hello_backend"
assert_match "Reinstalled code for canister hello_backend"
)
echo YeS | (
assert_command dfx deploy --mode=reinstall hello_backend

assert_match "YOU WILL LOSE ALL DATA IN THE CANISTER"
assert_match "Reinstalling code for canister hello_backend"
assert_match "Reinstalled code for canister hello_backend"
)
}
6 changes: 3 additions & 3 deletions e2e/tests-replica/deploy.bash
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ teardown() {
assert_command dfx canister create --all

assert_command dfx deploy
assert_match 'Installing code for canister'
assert_match 'Installed code for canister'
}

@test "dfx deploy supports arguments" {
Expand All @@ -85,13 +85,13 @@ teardown() {
# Therefore, there is no "attempting (install|upgrade)" message.

assert_command dfx deploy hello_backend
assert_match 'Installing code for canister'
assert_match 'Installed code for canister'

assert_command dfx canister call hello_backend greet '("First")'
assert_eq '("Hello, First!")'

assert_command dfx deploy hello_backend --upgrade-unchanged
assert_match 'Upgrading code for canister'
assert_match 'Upgraded code for canister'

assert_command dfx canister call hello_backend greet '("Second")'
assert_eq '("Hello, Second!")'
Expand Down
20 changes: 15 additions & 5 deletions src/dfx-core/src/canister/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::{
cli::ask_for_consent,
error::canister::{CanisterBuilderError, CanisterInstallError},
error::{
canister::{CanisterBuilderError, CanisterInstallError},
cli::UserConsent,
},
identity::CallSender,
};
use candid::Principal;
Expand Down Expand Up @@ -28,14 +30,22 @@ pub async fn build_wallet_canister(
.map_err(CanisterBuilderError::WalletCanisterCaller)
}

pub fn install_mode_to_prompt(mode: &InstallMode) -> &'static str {
pub fn install_mode_to_present_tense(mode: &InstallMode) -> &'static str {
match mode {
InstallMode::Install => "Installing",
InstallMode::Reinstall => "Reinstalling",
InstallMode::Upgrade { .. } => "Upgrading",
}
}

pub fn install_mode_to_past_tense(mode: &InstallMode) -> &'static str {
match mode {
InstallMode::Install => "Installed",
InstallMode::Reinstall => "Reinstalled",
InstallMode::Upgrade { .. } => "Upgraded",
}
}

pub async fn install_canister_wasm(
agent: &Agent,
canister_id: Principal,
Expand All @@ -44,10 +54,10 @@ pub async fn install_canister_wasm(
mode: InstallMode,
call_sender: &CallSender,
wasm_module: Vec<u8>,
skip_consent: bool,
ask_for_consent: impl FnOnce(&str) -> Result<(), UserConsent>,
) -> Result<(), CanisterInstallError> {
let mgr = ManagementCanister::create(agent);
if !skip_consent && mode == InstallMode::Reinstall {
if mode == InstallMode::Reinstall {
let msg = if let Some(name) = canister_name {
format!("You are about to reinstall the {name} canister")
} else {
Expand Down
17 changes: 0 additions & 17 deletions src/dfx-core/src/cli/mod.rs

This file was deleted.

1 change: 0 additions & 1 deletion src/dfx-core/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
pub mod canister;
pub mod cli;
pub mod config;
pub mod error;
pub mod extension;
Expand Down
4 changes: 2 additions & 2 deletions src/dfx/src/actors/pocketic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl Actor for PocketIc {
}

fn stopping(&mut self, _ctx: &mut Self::Context) -> Running {
warn!(self.logger, "Stopping PocketIC...");
debug!(self.logger, "Stopping PocketIC...");
if let Some(sender) = self.stop_sender.take() {
let _ = sender.send(());
}
Expand All @@ -168,7 +168,7 @@ impl Actor for PocketIc {
let _ = join.join();
}

warn!(self.logger, "Stopped.");
debug!(self.logger, "Stopped.");
Running::Stop
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/dfx/src/commands/canister/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ use crate::lib::operations::canister::{
use crate::lib::operations::cycles_ledger::wallet_deposit_to_cycles_ledger;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::assets::wallet_wasm;
use crate::util::blob_from_arguments;
use crate::util::clap::parsers::{cycle_amount_parser, icrc_subaccount_parser};
use crate::util::{ask_for_consent, blob_from_arguments};
use anyhow::{bail, Context};
use candid::Principal;
use clap::Parser;
use dfx_core::canister::build_wallet_canister;
use dfx_core::cli::ask_for_consent;
use dfx_core::identity::wallet::wallet_canister_id;
use dfx_core::identity::CallSender;
use fn_error_context::context;
Expand Down Expand Up @@ -177,9 +176,10 @@ async fn delete_canister(
// Determine how many cycles we can withdraw.
let status = canister::get_canister_status(env, canister_id, call_sender).await?;
if status.status != CanisterStatus::Stopped && !skip_confirmation {
ask_for_consent(&format!(
"Canister {canister} has not been stopped. Delete anyway?"
))?;
ask_for_consent(
env,
&format!("Canister {canister} has not been stopped. Delete anyway?"),
)?;
}
let agent = env.get_agent();
let mgr = ManagementCanister::create(agent);
Expand Down
34 changes: 26 additions & 8 deletions src/dfx/src/commands/canister/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::operations::canister::install_canister::install_canister;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::blob_from_arguments;
use crate::util::clap::argument_from_cli::ArgumentFromCliLongOpt;
use crate::util::clap::install_mode::{InstallModeHint, InstallModeOpt};
use dfx_core::canister::{install_canister_wasm, install_mode_to_prompt};
use crate::util::{ask_for_consent, blob_from_arguments};
use dfx_core::canister::{
install_canister_wasm, install_mode_to_past_tense, install_mode_to_present_tense,
};
use dfx_core::identity::CallSender;

use crate::lib::operations::canister::skip_remote_canister;
Expand Down Expand Up @@ -103,12 +105,15 @@ pub async fn exec(
)?;
let wasm_module = dfx_core::fs::read(wasm_path)?;
let mode = mode_hint.to_install_mode_with_wasm_path()?;
info!(
env.get_logger(),
"{} code for canister {}",
install_mode_to_prompt(&mode),
canister_id,
let spinner = env.new_spinner(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't play well with the "ask for consent" and interactive parameter prompting in that can happen when installing a canister. To repro, for example:

$ dfx canister install --wasm /Users/ericswanson/d/sdk/e2e/assets/wasm/identity/main.wasm --mode reinstall react_rust_backend
⠁ Reinstalling code for canister react_rust_backend, with canister ID bkyz2-fmaaa-aaaaa-qaaaq-cai                           WARNING!
You are about to reinstall the react_rust_backend canister
This will OVERWRITE all the data and code in the canister.

YOU WILL LOSE ALL DATA IN THE CANISTER.


Do you want to proceed? yes/No
⠋ Reinstalling code for canister react_rust_backend, with canister ID bkyz2-fmaaa-aaaaa-qaaaq-cai  

format!(
"{} code for canister {}",
install_mode_to_present_tense(&mode),
canister_id,
)
.into(),
);

install_canister_wasm(
env.get_agent(),
canister_id,
Expand All @@ -117,9 +122,22 @@ pub async fn exec(
mode,
call_sender,
wasm_module,
opts.yes,
|message| {
if opts.yes {
Ok(())
} else {
ask_for_consent(env, message)
}
},
)
.await?;
spinner.finish_and_clear();
info!(
env.get_logger(),
"{} code for canister {}",
install_mode_to_past_tense(&mode),
canister_id
);
Ok(())
} else {
bail!("When installing a canister by its ID, you must specify `--wasm` option.")
Expand Down
4 changes: 2 additions & 2 deletions src/dfx/src/commands/canister/update_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::lib::operations::canister::{
get_canister_status, skip_remote_canister, update_settings,
};
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::ask_for_consent;
use crate::util::clap::parsers::{
compute_allocation_parser, freezing_threshold_parser, memory_allocation_parser,
reserved_cycles_limit_parser, wasm_memory_limit_parser,
Expand All @@ -19,7 +20,6 @@ use byte_unit::Byte;
use candid::Principal as CanisterId;
use candid::Principal;
use clap::{ArgAction, Parser};
use dfx_core::cli::ask_for_consent;
use dfx_core::error::identity::InstantiateIdentityFromNameError::GetIdentityPrincipalFailed;
use dfx_core::identity::CallSender;
use fn_error_context::context;
Expand Down Expand Up @@ -146,7 +146,7 @@ pub async fn exec(
fetch_root_key_if_needed(env).await?;

if !opts.yes && user_is_removing_themselves_as_controller(env, call_sender, &opts)? {
ask_for_consent("You are trying to remove yourself as a controller of this canister. This may leave this canister un-upgradeable.")?
ask_for_consent(env, "You are trying to remove yourself as a controller of this canister. This may leave this canister un-upgradeable.")?
}

let controllers: Option<DfxResult<Vec<_>>> = opts.set_controller.as_ref().map(|controllers| {
Expand Down
2 changes: 1 addition & 1 deletion src/dfx/src/commands/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ fn display_urls(env: &dyn Environment) -> DfxResult {
info!(log, "URLs:");
let green = Style::new().green();
if !frontend_urls.is_empty() {
info!(log, " Frontend canister via browser");
info!(log, " Frontend canister via browser:");
for (name, (url1, url2)) in frontend_urls {
if let Some(url2) = url2 {
info!(log, " {}:", name);
Expand Down
5 changes: 2 additions & 3 deletions src/dfx/src/commands/wallet/redeem_faucet_coupon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::identity::wallet::set_wallet_id;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::{format_as_trillions, pretty_thousand_separators};
use crate::util::{ask_for_consent, format_as_trillions, pretty_thousand_separators};
use anyhow::{anyhow, bail, Context};
use candid::{encode_args, Decode, Principal};
use clap::Parser;
use dfx_core::cli::ask_for_consent;
use slog::{info, warn};

pub const DEFAULT_FAUCET_PRINCIPAL: Principal =
Expand Down Expand Up @@ -74,7 +73,7 @@ pub async fn exec(env: &dyn Environment, opts: RedeemFaucetCouponOpts) -> DfxRes
// identity has no wallet yet - faucet will provide one
_ => {
if !opts.yes {
ask_for_consent("`dfx cycles` is now recommended instead of `dfx wallet`. Are you sure you want to create a new cycles wallet anyway?")?;
ask_for_consent(env, "`dfx cycles` is now recommended instead of `dfx wallet`. Are you sure you want to create a new cycles wallet anyway?")?;
}
let identity = env
.get_selected_identity()
Expand Down
Loading
Loading