Skip to content

Commit

Permalink
trigger: Decouple Trigger from clap
Browse files Browse the repository at this point in the history
- Rename `Trigger`'s associated `CliArgs` type to `GlobalConfig`
- Move that `clap::Args` bound into `FactorsTriggerCommand`
- Fix up related code

Signed-off-by: Lann Martin <[email protected]>
  • Loading branch information
lann committed Sep 27, 2024
1 parent 416434b commit 3c86968
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 30 deletions.
10 changes: 5 additions & 5 deletions crates/trigger-http/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -52,7 +52,7 @@ pub struct CliArgs {
pub tls_key: Option<PathBuf>,
}

impl CliArgs {
impl GlobalConfig {
fn into_tls_config(self) -> Option<TlsConfig> {
match (self.tls_cert, self.tls_key) {
(Some(cert_path), Some(key_path)) => Some(TlsConfig {
Expand All @@ -78,11 +78,11 @@ pub struct HttpTrigger {
impl<F: RuntimeFactors> Trigger<F> 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> {
Self::new(app, cli_args.address, cli_args.into_tls_config())
fn new(cfg: Self::GlobalConfig, app: &spin_app::App) -> anyhow::Result<Self> {
Self::new(app, cfg.address, cfg.into_tls_config())
}

async fn run(self, trigger_app: TriggerApp<F>) -> anyhow::Result<()> {
Expand Down
6 changes: 3 additions & 3 deletions crates/trigger-redis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -34,11 +34,11 @@ struct TriggerConfig {
impl<F: RuntimeFactors> Trigger<F> 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<Self> {
fn new(_cfg: Self::GlobalConfig, _app: &App) -> anyhow::Result<Self> {
Ok(Self)
}

Expand Down
28 changes: 19 additions & 9 deletions crates/trigger/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ pub const SPIN_WORKING_DIR: &str = "SPIN_WORKING_DIR";
usage = "spin [COMMAND] [OPTIONS]",
next_help_heading = help_heading::<T, B::Factors>()
)]
pub struct FactorsTriggerCommand<T: Trigger<B::Factors>, B: RuntimeFactorsBuilder> {
pub struct FactorsTriggerCommand<T, B>
where
T: Trigger<B::Factors>,
T::GlobalConfig: Args,
B: RuntimeFactorsBuilder,
{
/// Log directory for the stdout and stderr of components. Setting to
/// the empty string disables logging to disk.
#[clap(
Expand Down Expand Up @@ -110,7 +115,7 @@ pub struct FactorsTriggerCommand<T: Trigger<B::Factors>, B: RuntimeFactorsBuilde
pub state_dir: Option<String>,

#[clap(flatten)]
pub trigger_args: T::CliArgs,
pub trigger_args: T::GlobalConfig,

#[clap(flatten)]
pub builder_args: B::CliArgs,
Expand Down Expand Up @@ -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<T: Trigger<B::Factors>, B: RuntimeFactorsBuilder> FactorsTriggerCommand<T, B> {
pub struct NoGlobalConfig;

impl<T, B> FactorsTriggerCommand<T, B>
where
T: Trigger<B::Factors>,
T::GlobalConfig: Args,
B: RuntimeFactorsBuilder,
{
/// Create a new TriggerExecutorBuilder from this TriggerExecutorCommand.
pub async fn run(self) -> Result<()> {
// Handle --help-args-only
Expand Down Expand Up @@ -383,10 +393,10 @@ pub mod help {

impl<F: RuntimeFactors> Trigger<F> 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<Self> {
fn new(_cfg: Self::GlobalConfig, _app: &App) -> anyhow::Result<Self> {
Ok(Self)
}

Expand Down
9 changes: 7 additions & 2 deletions crates/trigger/src/cli/launch_metadata.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clap::CommandFactory;
use clap::{Args, CommandFactory};
use serde::{Deserialize, Serialize};
use std::ffi::OsString;

Expand Down Expand Up @@ -29,7 +29,12 @@ struct LaunchFlag {
}

impl LaunchMetadata {
pub fn infer<T: Trigger<B::Factors>, B: RuntimeFactorsBuilder>() -> Self {
pub fn infer<T, B>() -> Self
where
T: Trigger<B::Factors>,
T::GlobalConfig: Args,
B: RuntimeFactorsBuilder,
{
let all_flags: Vec<_> = FactorsTriggerCommand::<T, B>::command()
.get_arguments()
.map(LaunchFlag::infer)
Expand Down
10 changes: 4 additions & 6 deletions crates/trigger/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`].
Expand All @@ -33,14 +31,14 @@ pub trait Trigger<F: RuntimeFactors>: 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<Self>;
fn new(cfg: Self::GlobalConfig, app: &App) -> anyhow::Result<Self>;

/// Update the [`spin_core::Config`] for this trigger.
///
Expand Down
8 changes: 4 additions & 4 deletions examples/spin-timer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -49,11 +49,11 @@ pub struct TimerTriggerConfig {
impl<F: RuntimeFactors> Trigger<F> 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<Self> {
fn new(cfg: Self::GlobalConfig, app: &App) -> anyhow::Result<Self> {
let trigger_type = <Self as Trigger<F>>::TYPE;
let metadata = app
.get_trigger_metadata::<TriggerMetadata>(trigger_type)?
Expand All @@ -67,7 +67,7 @@ impl<F: RuntimeFactors> Trigger<F> for TimerTrigger {
.collect();

Ok(Self {
test: cli_args.test,
test: cfg.test,
speedup,
component_timings,
})
Expand Down
2 changes: 1 addition & 1 deletion tests/testing-framework/src/runtimes/in_process_spin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down

0 comments on commit 3c86968

Please sign in to comment.