trigger crate prototype #415
Conversation
|
Note that this specific design would require a bit of refactoring in the http trigger's |
|
The |
| fn new( | ||
| execution_context: ExecutionContext<Self::ContextData>, | ||
| config: Self::Config, | ||
| component_configs: ComponentMap<Self::ComponentConfig>, |
There was a problem hiding this comment.
we can simply pass Application<CoreComponent> to new, instead of config and component_configs. Is this design intentionally trying to decouple Application and Trigger?
There was a problem hiding this comment.
Yes it's intentionally decoupling, but it also avoids needing to clone the Application, which seems to be part of the issue in #413.
There was a problem hiding this comment.
it seems like Http trigger will need application to build routers
| log_dir: opts.log_dir, | ||
| config_resolver: app.config_resolver, | ||
| })?; | ||
| T::configure_execution_context(&mut builder)?; |
There was a problem hiding this comment.
Maybe we should have one prepare trigger function that prepares builder, and then leave the Trigger::new and trigger.run to the up command.
pub async fn prepare_trigger<T: Trigger>(
app: Application<CoreComponent>,
opts: RunOptions<T>,
) -> (Builder<ContextData>, Config, ComponentConfig) {}This allows SDK users to customize builder and configs.
There was a problem hiding this comment.
In this PR's design the builder can be customized up in Trigger::configure_execution_context; that is just a default implementation of that trait function.
That said, I'm not opposed to leaving more of the logic in spin up at this point. This prototype was partially my attempt at figuring out what in implementation of #417 might look like if we move forward with that.
|
Implementation in #418 |
Currently just for discussion in #402