From 95f32dbff9b162dd0b11dac8bdf78d0bf85e5fe6 Mon Sep 17 00:00:00 2001 From: Vincent Zhang Date: Wed, 9 Oct 2024 11:32:49 +0800 Subject: [PATCH 1/6] Add PostStart actor after PocketIcProxy actor to output info messages. --- src/dfx/src/actors/mod.rs | 16 ++++++++ src/dfx/src/actors/pocketic_proxy.rs | 36 ++++++++++++++++-- src/dfx/src/actors/post_start.rs | 56 ++++++++++++++++++++++++++++ src/dfx/src/commands/start.rs | 15 ++++++-- 4 files changed, 116 insertions(+), 7 deletions(-) create mode 100644 src/dfx/src/actors/post_start.rs diff --git a/src/dfx/src/actors/mod.rs b/src/dfx/src/actors/mod.rs index ee44edd16b..22589041db 100644 --- a/src/dfx/src/actors/mod.rs +++ b/src/dfx/src/actors/mod.rs @@ -13,6 +13,7 @@ use dfx_core::config::model::replica_config::ReplicaConfig; use fn_error_context::context; use pocketic_proxy::signals::PortReadySubscribe; use pocketic_proxy::{PocketIcProxy, PocketIcProxyConfig}; +use post_start::PostStart; use std::fs; use std::path::PathBuf; @@ -22,6 +23,7 @@ pub mod btc_adapter; pub mod canister_http_adapter; pub mod pocketic; pub mod pocketic_proxy; +pub mod post_start; pub mod replica; mod shutdown; pub mod shutdown_controller; @@ -213,3 +215,17 @@ pub fn start_pocketic_actor( }; Ok(pocketic::PocketIc::new(actor_config).start()) } + +#[context("Failed to start PostStart actor.")] +pub fn start_post_start_actor( + env: &dyn Environment, + background: bool, + pocketic_proxy: Option>, +) -> DfxResult> { + let config = post_start::Config { + logger: env.get_logger().clone(), + background, + pocketic_proxy, + }; + Ok(PostStart::new(config).start()) +} diff --git a/src/dfx/src/actors/pocketic_proxy.rs b/src/dfx/src/actors/pocketic_proxy.rs index ba86f0d580..4f6da239bf 100644 --- a/src/dfx/src/actors/pocketic_proxy.rs +++ b/src/dfx/src/actors/pocketic_proxy.rs @@ -1,4 +1,5 @@ use crate::actors::pocketic_proxy::signals::{PortReadySignal, PortReadySubscribe}; +use crate::actors::post_start::signals::{PocketIcProxyReadySignal, PocketIcProxyReadySubscribe}; use crate::actors::shutdown::{wait_for_child_or_receiver, ChildOrReceiver}; use crate::actors::shutdown_controller::signals::outbound::Shutdown; use crate::actors::shutdown_controller::signals::ShutdownSubscribe; @@ -70,6 +71,9 @@ pub struct PocketIcProxy { stop_sender: Option>, thread_join: Option>, + + /// Ready Signal subscribers. + ready_subscribers: Vec>, } impl PocketIcProxy { @@ -81,10 +85,11 @@ impl PocketIcProxy { stop_sender: None, thread_join: None, logger, + ready_subscribers: Vec::new(), } } - fn start_pocketic_proxy(&mut self, replica_url: Url) -> DfxResult { + fn start_pocketic_proxy(&mut self, replica_url: Url, addr: Addr) -> DfxResult { let logger = self.logger.clone(); let config = &self.config.pocketic_proxy_config; let pocketic_proxy_path = self.config.pocketic_proxy_path.clone(); @@ -100,6 +105,7 @@ impl PocketIcProxy { pocketic_proxy_path, pocketic_proxy_pid_path, pocketic_proxy_port_path, + addr, receiver, config.verbose, config.domains.clone(), @@ -164,7 +170,7 @@ impl Actor for PocketIcProxy { .do_send(ShutdownSubscribe(ctx.address().recipient::())); if let Some(replica_url) = &self.config.pocketic_proxy_config.replica_url { - self.start_pocketic_proxy(replica_url.clone()) + self.start_pocketic_proxy(replica_url.clone(), ctx.address()) .expect("Could not start PocketIC HTTP gateway"); } } @@ -179,7 +185,7 @@ impl Actor for PocketIcProxy { impl Handler for PocketIcProxy { type Result = (); - fn handle(&mut self, msg: PortReadySignal, _ctx: &mut Self::Context) { + fn handle(&mut self, msg: PortReadySignal, ctx: &mut Self::Context) { debug!( self.logger, "replica ready on {}, so re/starting HTTP gateway", msg.url @@ -189,11 +195,29 @@ impl Handler for PocketIcProxy { let replica_url = Url::parse(&msg.url).unwrap(); - self.start_pocketic_proxy(replica_url) + self.start_pocketic_proxy(replica_url, ctx.address()) .expect("Could not start PocketIC HTTP gateway"); } } +impl Handler for PocketIcProxy { + type Result = (); + + fn handle(&mut self, msg: PocketIcProxyReadySubscribe, _ctx: &mut Self::Context) { + self.ready_subscribers.push(msg.0); + } +} + +impl Handler for PocketIcProxy { + type Result = (); + + fn handle(&mut self, _msg: PocketIcProxyReadySignal, _ctx: &mut Self::Context) { + for sub in &self.ready_subscribers { + sub.do_send(PocketIcProxyReadySignal); + } + } +} + impl Handler for PocketIcProxy { type Result = ResponseActFuture>; @@ -217,6 +241,7 @@ fn pocketic_proxy_start_thread( pocketic_proxy_path: PathBuf, pocketic_proxy_pid_path: PathBuf, pocketic_proxy_port_path: PathBuf, + addr: Addr, receiver: Receiver<()>, verbose: bool, domains: Option>, @@ -277,6 +302,9 @@ fn pocketic_proxy_start_thread( } info!(logger, "Replica API running on {address}"); + // Send PocketIcProxyReadySignal to PocketIcProxy. + addr.do_send(PocketIcProxyReadySignal); + // This waits for the child to stop, or the receiver to receive a message. // We don't restart pocket-ic if done = true. match wait_for_child_or_receiver(&mut child, &receiver) { diff --git a/src/dfx/src/actors/post_start.rs b/src/dfx/src/actors/post_start.rs new file mode 100644 index 0000000000..ff4971b3af --- /dev/null +++ b/src/dfx/src/actors/post_start.rs @@ -0,0 +1,56 @@ +use crate::actors::post_start::signals::{PocketIcProxyReadySignal, PocketIcProxyReadySubscribe}; +use crate::actors::pocketic_proxy::PocketIcProxy; +use actix::{Actor, Addr, AsyncContext, Context, Handler}; +use slog::{info, Logger}; + +pub mod signals { + use actix::prelude::*; + + #[derive(Message)] + #[rtype(result = "()")] + pub struct PocketIcProxyReadySignal; + + #[derive(Message)] + #[rtype(result = "()")] + pub struct PocketIcProxyReadySubscribe(pub Recipient); +} + +pub struct Config { + pub logger: Logger, + pub background: bool, + pub pocketic_proxy: Option>, +} + +pub struct PostStart { + config: Config, +} + +impl PostStart { + pub fn new(config: Config) -> Self { + Self { config } + } +} + +impl Actor for PostStart { + type Context = Context; + + fn started(&mut self, ctx: &mut Self::Context) { + // Register the PostStart recipent to PocketIcProxy. + if let Some(pocketic_proxy) = &self.config.pocketic_proxy { + pocketic_proxy.do_send(PocketIcProxyReadySubscribe(ctx.address().recipient())); + } + } +} + +impl Handler for PostStart { + type Result = (); + + fn handle(&mut self, _msg: PocketIcProxyReadySignal, _ctx: &mut Self::Context) -> Self::Result { + let logger = &self.config.logger; + if self.config.background { + info!(logger, "The dfx server is running in the background.") + } else { + info!(logger, "The dfx server is running.\nYou can start a new terminal to continue developing, or quit with 'Ctrl-C'."); + } + } +} diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index b5f96855f5..2a44dbb7bb 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -1,7 +1,8 @@ use crate::actors::pocketic_proxy::{signals::PortReadySubscribe, PocketIcProxyConfig}; use crate::actors::{ start_btc_adapter_actor, start_canister_http_adapter_actor, start_pocketic_actor, - start_pocketic_proxy_actor, start_replica_actor, start_shutdown_controller, + start_pocketic_proxy_actor, start_post_start_actor, start_replica_actor, + start_shutdown_controller, }; use crate::config::dfx_version_str; use crate::error_invalid_argument; @@ -264,6 +265,10 @@ pub fn exec( } local_server_descriptor.describe(env.get_logger()); + // Get the original background flag set by the user from the command arguments. + // Get it from the environment variable as the `--background` flag will be ignored by the send_background() method. + let original_background = std::env::var("original_background").is_ok(); + write_pid(&pid_file_path); fs::write(&webserver_port_path, address_and_port.port().to_string())?; @@ -413,7 +418,10 @@ pub fn exec( pocketic_proxy_pid_file_path, pocketic_proxy_port_file_path, )?; - Ok::<_, Error>(proxy) + + let post_start = start_post_start_actor(env, original_background, Some(proxy))?; + + Ok::<_, Error>(post_start) })?; system.run()?; @@ -574,7 +582,8 @@ fn send_background() -> DfxResult<()> { .skip(1) .filter(|a| !a.eq("--background")) .filter(|a| !a.eq("--clean")), - ); + ) + .env("original_background", "true"); // Set the `original_background` environment variable which will be used by the second start. cmd.spawn().context("Failed to spawn child process.")?; Ok(()) From 0a0447f558051743ac4aab6b06bc09950c71ca85 Mon Sep 17 00:00:00 2001 From: Vincent Zhang Date: Wed, 9 Oct 2024 11:55:09 +0800 Subject: [PATCH 2/6] Fix format. --- src/dfx/src/actors/post_start.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dfx/src/actors/post_start.rs b/src/dfx/src/actors/post_start.rs index ff4971b3af..8c9145e553 100644 --- a/src/dfx/src/actors/post_start.rs +++ b/src/dfx/src/actors/post_start.rs @@ -1,5 +1,5 @@ -use crate::actors::post_start::signals::{PocketIcProxyReadySignal, PocketIcProxyReadySubscribe}; use crate::actors::pocketic_proxy::PocketIcProxy; +use crate::actors::post_start::signals::{PocketIcProxyReadySignal, PocketIcProxyReadySubscribe}; use actix::{Actor, Addr, AsyncContext, Context, Handler}; use slog::{info, Logger}; From 153d344a5b1cca0a504ef1571ed1aac79142f437 Mon Sep 17 00:00:00 2001 From: Vincent Zhang Date: Mon, 21 Oct 2024 12:50:37 +0800 Subject: [PATCH 3/6] Add tests. --- e2e/tests-dfx/start.bash | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/e2e/tests-dfx/start.bash b/e2e/tests-dfx/start.bash index 3b6fe8207c..658bd48968 100644 --- a/e2e/tests-dfx/start.bash +++ b/e2e/tests-dfx/start.bash @@ -60,7 +60,8 @@ teardown() { } @test "start and stop outside project" { - dfx_start + assert_command dfx_start + assert_contains "The dfx server is running in the background." mkdir subdir cd subdir || exit 1 From 2ab991b1613b901a993a18b7172ebcb6df9b76c0 Mon Sep 17 00:00:00 2001 From: Vincent Zhang Date: Mon, 21 Oct 2024 12:55:17 +0800 Subject: [PATCH 4/6] Update changelog. --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e37c6b50a..ff21391ede 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,14 @@ Valid settings are: The frontend canister sync now tries to batch multiple small content chunks into a single call using the `create_chunks` method added earlier. This should lead to significantly faster upload times for frontends with many small files. +### chore!: improve `dfx start` messages. + +For `dfx start`, show below messages to users to indicate what to do next. +``` +The dfx server is running. +You can start a new terminal to continue developing, or quit with 'Ctrl-C'. +``` + ## Dependencies ### Frontend canister From 060b25638ae42ecbf2c5854962084a6c5851781e Mon Sep 17 00:00:00 2001 From: Vincent Zhang Date: Thu, 7 Nov 2024 20:20:46 +0800 Subject: [PATCH 5/6] Addressed review comments. --- CHANGELOG.md | 16 ++++++++-------- e2e/tests-dfx/start.bash | 2 +- src/dfx/src/actors/post_start.rs | 4 ++-- src/dfx/src/commands/start.rs | 13 +++++++------ 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd62a127e6..5a90ebc2ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,14 @@ Allow setting permissions lists in init arguments just like in upgrade arguments - Module hash: f45db224b40fac516c877e3108dc809d4b22fa42d05ee8dfa5002536a3a3daed - Bump agent-js to fix error code +### chore: improve `dfx start` messages. + +For `dfx start`, show below messages to users to indicate what to do next. +``` +Success! The dfx server is running. +You must open a new terminal to continue developing. If you'd prefer to stop, quit with 'Ctrl-C'. +``` + # 0.24.2 ### feat: Support canister log allowed viewer list @@ -32,14 +40,6 @@ The frontend canister sync now tries to batch multiple small content chunks into And for small amounts of uploaded data the asset sync can now skip chunk creation entirely. This should lead to significantly faster upload times for frontends with many small files. -### chore!: improve `dfx start` messages. - -For `dfx start`, show below messages to users to indicate what to do next. -``` -The dfx server is running. -You can start a new terminal to continue developing, or quit with 'Ctrl-C'. -``` - ## Dependencies ### Motoko diff --git a/e2e/tests-dfx/start.bash b/e2e/tests-dfx/start.bash index 658bd48968..bdfd7d25d6 100644 --- a/e2e/tests-dfx/start.bash +++ b/e2e/tests-dfx/start.bash @@ -61,7 +61,7 @@ teardown() { @test "start and stop outside project" { assert_command dfx_start - assert_contains "The dfx server is running in the background." + assert_contains "Success! The dfx server is running in the background." mkdir subdir cd subdir || exit 1 diff --git a/src/dfx/src/actors/post_start.rs b/src/dfx/src/actors/post_start.rs index 8c9145e553..1a465cf24a 100644 --- a/src/dfx/src/actors/post_start.rs +++ b/src/dfx/src/actors/post_start.rs @@ -48,9 +48,9 @@ impl Handler for PostStart { fn handle(&mut self, _msg: PocketIcProxyReadySignal, _ctx: &mut Self::Context) -> Self::Result { let logger = &self.config.logger; if self.config.background { - info!(logger, "The dfx server is running in the background.") + info!(logger, "Success! The dfx server is running in the background.") } else { - info!(logger, "The dfx server is running.\nYou can start a new terminal to continue developing, or quit with 'Ctrl-C'."); + info!(logger, "Success! The dfx server is running.\nYou must open a new terminal to continue developing. If you'd prefer to stop, quit with 'Ctrl-C'."); } } } diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 32a34a31a4..909058dfa4 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -50,6 +50,10 @@ pub struct StartOpts { #[arg(long)] background: bool, + /// Indicates if the actual dfx process is running in the background. + #[arg(long, env = "DFX_RUNNING_IN_BACKGROUND", hide = true)] + running_in_background: bool, + /// Cleans the state of the current project. #[arg(long)] clean: bool, @@ -139,6 +143,7 @@ pub fn exec( StartOpts { host, background, + running_in_background, clean, force, bitcoin_node, @@ -265,10 +270,6 @@ pub fn exec( } local_server_descriptor.describe(env.get_logger()); - // Get the original background flag set by the user from the command arguments. - // Get it from the environment variable as the `--background` flag will be ignored by the send_background() method. - let original_background = std::env::var("original_background").is_ok(); - write_pid(&pid_file_path); fs::write(&webserver_port_path, address_and_port.port().to_string())?; @@ -419,7 +420,7 @@ pub fn exec( pocketic_proxy_port_file_path, )?; - let post_start = start_post_start_actor(env, original_background, Some(proxy))?; + let post_start = start_post_start_actor(env, running_in_background, Some(proxy))?; Ok::<_, Error>(post_start) })?; @@ -583,7 +584,7 @@ fn send_background() -> DfxResult<()> { .filter(|a| !a.eq("--background")) .filter(|a| !a.eq("--clean")), ) - .env("original_background", "true"); // Set the `original_background` environment variable which will be used by the second start. + .env("DFX_RUNNING_IN_BACKGROUND", "true"); // Set the `DFX_RUNNING_IN_BACKGROUND` environment variable which will be used by the second start. cmd.spawn().context("Failed to spawn child process.")?; Ok(()) From 47d62d76929176c59c71b3f0941aa9676eeebfbf Mon Sep 17 00:00:00 2001 From: Vincent Zhang Date: Thu, 7 Nov 2024 20:49:21 +0800 Subject: [PATCH 6/6] Fix format. --- src/dfx/src/actors/post_start.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/dfx/src/actors/post_start.rs b/src/dfx/src/actors/post_start.rs index 1a465cf24a..35222e6e57 100644 --- a/src/dfx/src/actors/post_start.rs +++ b/src/dfx/src/actors/post_start.rs @@ -48,9 +48,15 @@ impl Handler for PostStart { fn handle(&mut self, _msg: PocketIcProxyReadySignal, _ctx: &mut Self::Context) -> Self::Result { let logger = &self.config.logger; if self.config.background { - info!(logger, "Success! The dfx server is running in the background.") + info!( + logger, + "Success! The dfx server is running in the background." + ) } else { - info!(logger, "Success! The dfx server is running.\nYou must open a new terminal to continue developing. If you'd prefer to stop, quit with 'Ctrl-C'."); + info!( + logger, + "Success! The dfx server is running.\nYou must open a new terminal to continue developing. If you'd prefer to stop, quit with 'Ctrl-C'." + ) } } }