From 416434be97d66658f8f867bbd647166618e89e04 Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Wed, 25 Sep 2024 08:37:11 -0400 Subject: [PATCH 1/2] trigger: Re-export a few things to ease trigger development Signed-off-by: Lann Martin --- crates/trigger/src/lib.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/trigger/src/lib.rs b/crates/trigger/src/lib.rs index daefa659b..089df3772 100644 --- a/crates/trigger/src/lib.rs +++ b/crates/trigger/src/lib.rs @@ -4,10 +4,12 @@ pub mod loader; use std::future::Future; use clap::Args; -use spin_core::Linker; -use spin_factors::RuntimeFactors; +pub use spin_core::Linker; +pub use spin_factors::RuntimeFactors; use spin_factors_executor::{FactorsExecutorApp, FactorsInstanceBuilder}; +pub use anyhow; +pub use clap::Parser; pub use spin_app::App; /// Type alias for a [`spin_factors_executor::FactorsExecutorApp`] specialized to a [`Trigger`]. @@ -21,7 +23,7 @@ pub type TriggerInstanceBuilder<'a, T, F> = pub type Store = spin_core::Store>; /// Type alias for [`spin_factors_executor::InstanceState`] specialized to a [`Trigger`]. -type TriggerInstanceState = spin_factors_executor::InstanceState< +pub type TriggerInstanceState = spin_factors_executor::InstanceState< ::InstanceState, >::InstanceState, >; From 3c86968ad5cfc6bba194504938acc610e909e4a1 Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Fri, 27 Sep 2024 11:16:45 -0400 Subject: [PATCH 2/2] trigger: Decouple Trigger from clap - Rename `Trigger`'s associated `CliArgs` type to `GlobalConfig` - Move that `clap::Args` bound into `FactorsTriggerCommand` - Fix up related code Signed-off-by: Lann Martin --- crates/trigger-http/src/lib.rs | 10 +++---- crates/trigger-redis/src/lib.rs | 6 ++-- crates/trigger/src/cli.rs | 28 +++++++++++++------ crates/trigger/src/cli/launch_metadata.rs | 9 ++++-- crates/trigger/src/lib.rs | 10 +++---- examples/spin-timer/src/lib.rs | 8 +++--- .../src/runtimes/in_process_spin.rs | 2 +- 7 files changed, 43 insertions(+), 30 deletions(-) diff --git a/crates/trigger-http/src/lib.rs b/crates/trigger-http/src/lib.rs index bc004dc0d..570974029 100644 --- a/crates/trigger-http/src/lib.rs +++ b/crates/trigger-http/src/lib.rs @@ -38,7 +38,7 @@ pub(crate) type TriggerInstanceBuilder<'a, F> = spin_trigger::TriggerInstanceBuilder<'a, HttpTrigger, F>; #[derive(Args)] -pub struct CliArgs { +pub struct GlobalConfig { /// IP address and port to listen on #[clap(long = "listen", env = "SPIN_HTTP_LISTEN_ADDR", default_value = "127.0.0.1:3000", value_parser = parse_listen_addr)] pub address: SocketAddr, @@ -52,7 +52,7 @@ pub struct CliArgs { pub tls_key: Option, } -impl CliArgs { +impl GlobalConfig { fn into_tls_config(self) -> Option { match (self.tls_cert, self.tls_key) { (Some(cert_path), Some(key_path)) => Some(TlsConfig { @@ -78,11 +78,11 @@ pub struct HttpTrigger { impl Trigger for HttpTrigger { const TYPE: &'static str = "http"; - type CliArgs = CliArgs; + type GlobalConfig = GlobalConfig; type InstanceState = (); - fn new(cli_args: Self::CliArgs, app: &spin_app::App) -> anyhow::Result { - Self::new(app, cli_args.address, cli_args.into_tls_config()) + fn new(cfg: Self::GlobalConfig, app: &spin_app::App) -> anyhow::Result { + Self::new(app, cfg.address, cfg.into_tls_config()) } async fn run(self, trigger_app: TriggerApp) -> anyhow::Result<()> { diff --git a/crates/trigger-redis/src/lib.rs b/crates/trigger-redis/src/lib.rs index 9e50a74b8..3ea8d9a08 100644 --- a/crates/trigger-redis/src/lib.rs +++ b/crates/trigger-redis/src/lib.rs @@ -6,7 +6,7 @@ use redis::{Client, Msg}; use serde::Deserialize; use spin_factor_variables::VariablesFactor; use spin_factors::RuntimeFactors; -use spin_trigger::{cli::NoCliArgs, App, Trigger, TriggerApp}; +use spin_trigger::{cli::NoGlobalConfig, App, Trigger, TriggerApp}; use spin_world::exports::fermyon::spin::inbound_redis; use tracing::{instrument, Level}; @@ -34,11 +34,11 @@ struct TriggerConfig { impl Trigger for RedisTrigger { const TYPE: &'static str = "redis"; - type CliArgs = NoCliArgs; + type GlobalConfig = NoGlobalConfig; type InstanceState = (); - fn new(_cli_args: Self::CliArgs, _app: &App) -> anyhow::Result { + fn new(_cfg: Self::GlobalConfig, _app: &App) -> anyhow::Result { Ok(Self) } diff --git a/crates/trigger/src/cli.rs b/crates/trigger/src/cli.rs index 720ff1bdd..bac005c7c 100644 --- a/crates/trigger/src/cli.rs +++ b/crates/trigger/src/cli.rs @@ -41,7 +41,12 @@ pub const SPIN_WORKING_DIR: &str = "SPIN_WORKING_DIR"; usage = "spin [COMMAND] [OPTIONS]", next_help_heading = help_heading::() )] -pub struct FactorsTriggerCommand, B: RuntimeFactorsBuilder> { +pub struct FactorsTriggerCommand +where + T: Trigger, + T::GlobalConfig: Args, + B: RuntimeFactorsBuilder, +{ /// Log directory for the stdout and stderr of components. Setting to /// the empty string disables logging to disk. #[clap( @@ -110,7 +115,7 @@ pub struct FactorsTriggerCommand, B: RuntimeFactorsBuilde pub state_dir: Option, #[clap(flatten)] - pub trigger_args: T::CliArgs, + pub trigger_args: T::GlobalConfig, #[clap(flatten)] pub builder_args: B::CliArgs, @@ -139,12 +144,17 @@ pub struct FactorsConfig { pub log_dir: UserProvidedPath, } -/// An empty implementation of clap::Args to be used as TriggerExecutor::RunConfig -/// for executors that do not need additional CLI args. +/// This type may be used as the [`Trigger::GlobalConfig`] for triggers with no +/// CLI args. #[derive(Args)] -pub struct NoCliArgs; - -impl, B: RuntimeFactorsBuilder> FactorsTriggerCommand { +pub struct NoGlobalConfig; + +impl FactorsTriggerCommand +where + T: Trigger, + T::GlobalConfig: Args, + B: RuntimeFactorsBuilder, +{ /// Create a new TriggerExecutorBuilder from this TriggerExecutorCommand. pub async fn run(self) -> Result<()> { // Handle --help-args-only @@ -383,10 +393,10 @@ pub mod help { impl Trigger for HelpArgsOnlyTrigger { const TYPE: &'static str = "help-args-only"; - type CliArgs = NoCliArgs; + type GlobalConfig = NoGlobalConfig; type InstanceState = (); - fn new(_cli_args: Self::CliArgs, _app: &App) -> anyhow::Result { + fn new(_cfg: Self::GlobalConfig, _app: &App) -> anyhow::Result { Ok(Self) } diff --git a/crates/trigger/src/cli/launch_metadata.rs b/crates/trigger/src/cli/launch_metadata.rs index cbcab49f9..7ec5e87c4 100644 --- a/crates/trigger/src/cli/launch_metadata.rs +++ b/crates/trigger/src/cli/launch_metadata.rs @@ -1,4 +1,4 @@ -use clap::CommandFactory; +use clap::{Args, CommandFactory}; use serde::{Deserialize, Serialize}; use std::ffi::OsString; @@ -29,7 +29,12 @@ struct LaunchFlag { } impl LaunchMetadata { - pub fn infer, B: RuntimeFactorsBuilder>() -> Self { + pub fn infer() -> Self + where + T: Trigger, + T::GlobalConfig: Args, + B: RuntimeFactorsBuilder, + { let all_flags: Vec<_> = FactorsTriggerCommand::::command() .get_arguments() .map(LaunchFlag::infer) diff --git a/crates/trigger/src/lib.rs b/crates/trigger/src/lib.rs index 089df3772..b13d315f8 100644 --- a/crates/trigger/src/lib.rs +++ b/crates/trigger/src/lib.rs @@ -3,13 +3,11 @@ pub mod loader; use std::future::Future; -use clap::Args; pub use spin_core::Linker; pub use spin_factors::RuntimeFactors; use spin_factors_executor::{FactorsExecutorApp, FactorsInstanceBuilder}; pub use anyhow; -pub use clap::Parser; pub use spin_app::App; /// Type alias for a [`spin_factors_executor::FactorsExecutorApp`] specialized to a [`Trigger`]. @@ -33,14 +31,14 @@ pub trait Trigger: Sized + Send { /// A unique identifier for this trigger. const TYPE: &'static str; - /// The specific CLI arguments for this trigger. - type CliArgs: Args; + /// Global configuration used to construct this trigger. + type GlobalConfig; - /// The instance state for this trigger. + /// Extra state attached to each instance store for this trigger. type InstanceState: Send + 'static; /// Constructs a new trigger. - fn new(cli_args: Self::CliArgs, app: &App) -> anyhow::Result; + fn new(cfg: Self::GlobalConfig, app: &App) -> anyhow::Result; /// Update the [`spin_core::Config`] for this trigger. /// diff --git a/examples/spin-timer/src/lib.rs b/examples/spin-timer/src/lib.rs index ae18f9e30..7c3d18014 100644 --- a/examples/spin-timer/src/lib.rs +++ b/examples/spin-timer/src/lib.rs @@ -12,7 +12,7 @@ wasmtime::component::bindgen!({ }); #[derive(Args)] -pub struct CliArgs { +pub struct GlobalConfig { /// If true, run each component once and exit #[clap(long)] pub test: bool, @@ -49,11 +49,11 @@ pub struct TimerTriggerConfig { impl Trigger for TimerTrigger { const TYPE: &'static str = "timer"; - type CliArgs = CliArgs; + type GlobalConfig = GlobalConfig; type InstanceState = (); - fn new(cli_args: Self::CliArgs, app: &App) -> anyhow::Result { + fn new(cfg: Self::GlobalConfig, app: &App) -> anyhow::Result { let trigger_type = >::TYPE; let metadata = app .get_trigger_metadata::(trigger_type)? @@ -67,7 +67,7 @@ impl Trigger for TimerTrigger { .collect(); Ok(Self { - test: cli_args.test, + test: cfg.test, speedup, component_timings, }) diff --git a/tests/testing-framework/src/runtimes/in_process_spin.rs b/tests/testing-framework/src/runtimes/in_process_spin.rs index b9320e68c..5debd42db 100644 --- a/tests/testing-framework/src/runtimes/in_process_spin.rs +++ b/tests/testing-framework/src/runtimes/in_process_spin.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use anyhow::Context as _; -use spin_runtime_factors::{FactorsBuilder, TriggerAppArgs, TriggerFactors}; +use spin_runtime_factors::{FactorsBuilder, TriggerFactors, TriggerAppArgs}; use spin_trigger::{cli::TriggerAppBuilder, loader::ComponentLoader}; use spin_trigger_http::{HttpServer, HttpTrigger}; use test_environment::{