From 1e6f53b92cbee395b9b83e0d1a9a02b5c44c9cc4 Mon Sep 17 00:00:00 2001 From: Samir Talwar Date: Tue, 27 Feb 2024 07:20:20 +0100 Subject: [PATCH] Split out setup methods into their own trait. (#111) These now receive access to `&self`, allowing us to inject further information into configuration. A `default_main_with` function lets us provide alternative setups. --- rust-connector-sdk/src/connector.rs | 96 +++++++++++---------- rust-connector-sdk/src/connector/example.rs | 19 ++-- rust-connector-sdk/src/default_main.rs | 86 +++++++++++------- 3 files changed, 117 insertions(+), 84 deletions(-) diff --git a/rust-connector-sdk/src/connector.rs b/rust-connector-sdk/src/connector.rs index 457c1ddf..bb228ecc 100644 --- a/rust-connector-sdk/src/connector.rs +++ b/rust-connector-sdk/src/connector.rs @@ -221,44 +221,35 @@ pub enum MutationError { /// Connectors using this library should implement this trait. /// +/// It provides methods which implement the standard endpoints defined by the +/// specification: capabilities, schema, query, mutation, query/explain, +/// and mutation/explain. /// -/// It provides methods which implement the standard endpoints -/// defined by the specification: capabilities, schema, query, mutation, -/// query/explain, and mutation/explain. -/// -/// In addition, it introduces names for types to manage -/// state and configuration (if any), and provides any necessary context -/// for observability purposes (metrics, logging and tracing). +/// In addition, it introduces names for types to manage state and configuration +/// (if any), and provides any necessary context for observability purposes +/// (metrics, logging and tracing). /// /// ## Configuration /// /// Connectors encapsulate data sources, and likely require configuration -/// (connection strings, web service tokens, etc.). The NDC specification -/// does not discuss this sort of configuration, because it is an -/// implementation detail of a specific connector, but it is useful to -/// adopt a convention here for simplified configuration management. -/// -/// Configuration is specified as JSON, validated, and stored in a binary -/// format. +/// (connection strings, web service tokens, etc.). The NDC specification does +/// not discuss this sort of configuration, because it is an implementation +/// detail of a specific connector, but it is useful to adopt a convention here +/// for simplified configuration management. /// -/// This trait defines two types for managing configuration: -/// -/// - [`Connector::RawConfiguration`] defines the type of unvalidated, raw -/// configuration. -/// - [`Connector::Configuration`] defines the type of validated -/// configuration. Ideally, invalid configuration should not be representable -/// in this form. +/// Configuration is specified by the connector implementation. It is provided +/// as a path to a directory. Parsing this directory should result in a +/// [`Connector::Configuration`]. /// /// ## State /// -/// In addition to configuration, this trait defines a type for state management: -/// -/// - [`Connector::State`] defines the type of any unserializable runtime state. +/// In addition to configuration, this trait defines a [`Connector::State`] type +/// for state management. /// -/// State is distinguished from configuration in that it is not provided directly by -/// the user, and would not ordinarily be serializable. For example, a connection string -/// would be configuration, but a connection pool object created from that -/// connection string would be state. +/// State is distinguished from configuration in that it is not provided +/// directly by the user, and would not ordinarily be serializable. For example, +/// a connection string would be configuration, but a connection pool object +/// created from that connection string would be state. #[async_trait] pub trait Connector { /// The type of validated configuration @@ -266,24 +257,6 @@ pub trait Connector { /// The type of unserializable state type State: Sync + Send; - /// Validate the raw configuration provided by the user, - /// returning a configuration error or a validated [`Connector::Configuration`]. - async fn parse_configuration( - configuration_dir: impl AsRef + Send, - ) -> Result; - - /// Initialize the connector's in-memory state. - /// - /// For example, any connection pools, prepared queries, - /// or other managed resources would be allocated here. - /// - /// In addition, this function should register any - /// connector-specific metrics with the metrics registry. - async fn try_init_state( - configuration: &Self::Configuration, - metrics: &mut prometheus::Registry, - ) -> Result; - /// Update any metrics from the state /// /// Note: some metrics can be updated directly, and do not @@ -359,3 +332,34 @@ pub trait Connector { request: models::QueryRequest, ) -> Result, QueryError>; } + +/// Connectors are set up by values that implement this trait. +/// +/// It provides a method for parsing configuration, and another for initializing +/// state. +/// +/// See [`Connector`] for further details. +#[async_trait] +pub trait ConnectorSetup { + type Connector: Connector; + + /// Validate the configuration provided by the user, returning a + /// configuration error or a validated [`Connector::Configuration`]. + async fn parse_configuration( + &self, + configuration_dir: impl AsRef + Send, + ) -> Result<::Configuration, ParseError>; + + /// Initialize the connector's in-memory state. + /// + /// For example, any connection pools, prepared queries, or other managed + /// resources would be allocated here. + /// + /// In addition, this function should register any connector-specific + /// metrics with the metrics registry. + async fn try_init_state( + &self, + configuration: &::Configuration, + metrics: &mut prometheus::Registry, + ) -> Result<::State, InitializationError>; +} diff --git a/rust-connector-sdk/src/connector/example.rs b/rust-connector-sdk/src/connector/example.rs index d55f23af..2ae444cc 100644 --- a/rust-connector-sdk/src/connector/example.rs +++ b/rust-connector-sdk/src/connector/example.rs @@ -10,22 +10,29 @@ use super::*; pub struct Example {} #[async_trait] -impl Connector for Example { - type Configuration = (); - type State = (); +impl ConnectorSetup for Example { + type Connector = Self; async fn parse_configuration( + &self, _configuration_dir: impl AsRef + Send, - ) -> Result { + ) -> Result<::Configuration, ParseError> { Ok(()) } async fn try_init_state( - _configuration: &Self::Configuration, + &self, + _configuration: &::Configuration, _metrics: &mut prometheus::Registry, - ) -> Result { + ) -> Result<::State, InitializationError> { Ok(()) } +} + +#[async_trait] +impl Connector for Example { + type Configuration = (); + type State = (); fn fetch_metrics( _configuration: &Self::Configuration, diff --git a/rust-connector-sdk/src/default_main.rs b/rust-connector-sdk/src/default_main.rs index 29f0804a..25cd644d 100644 --- a/rust-connector-sdk/src/default_main.rs +++ b/rust-connector-sdk/src/default_main.rs @@ -26,7 +26,7 @@ use ndc_client::models::{ use ndc_test::report; use crate::check_health; -use crate::connector::Connector; +use crate::connector::{Connector, ConnectorSetup}; use crate::json_rejection::JsonRejection; use crate::json_response::JsonResponse; use crate::routes; @@ -158,34 +158,52 @@ impl ServerState { /// - It reads configuration as JSON from a file specified on the command line, /// - It reports traces to an OTLP collector specified on the command line, /// - Logs are written to stdout -pub async fn default_main() -> Result<(), Box> +pub async fn default_main() -> Result<(), Box> where - C::Configuration: Clone, - C::State: Clone, + Setup: ConnectorSetup + Default, + Setup::Connector: Connector + 'static, + ::Configuration: Clone, + ::State: Clone, +{ + default_main_with(Setup::default()).await +} + +/// A default main function for a connector, with a non-default setup. +/// +/// See [`default_main`] for further details. +pub async fn default_main_with(setup: Setup) -> Result<(), Box> +where + Setup: ConnectorSetup, + Setup::Connector: Connector + 'static, + ::Configuration: Clone, + ::State: Clone, { let CliArgs { command } = CliArgs::parse(); match command { - Command::Serve(serve_command) => serve::(serve_command).await, - Command::Test(test_command) => test::(test_command).await, - Command::Replay(replay_command) => replay::(replay_command).await, + Command::Serve(serve_command) => serve(setup, serve_command).await, + Command::Test(test_command) => test(setup, test_command).await, + Command::Replay(replay_command) => replay(setup, replay_command).await, Command::CheckHealth(check_health_command) => check_health(check_health_command).await, } } -async fn serve( +async fn serve( + setup: Setup, serve_command: ServeCommand, ) -> Result<(), Box> where - C::Configuration: Clone, - C::State: Clone, + Setup: ConnectorSetup, + Setup::Connector: Connector + 'static, + ::Configuration: Clone, + ::State: Clone, { init_tracing(&serve_command.service_name, &serve_command.otlp_endpoint) .expect("Unable to initialize tracing"); - let server_state = init_server_state::(serve_command.configuration).await?; + let server_state = init_server_state(setup, serve_command.configuration).await?; - let router = create_router::( + let router = create_router::( server_state.clone(), serve_command.service_token_secret.clone(), ); @@ -238,20 +256,19 @@ where } /// Initialize the server state from the configuration file. -pub async fn init_server_state( +pub async fn init_server_state( + setup: Setup, config_directory: impl AsRef + Send, -) -> Result, Box> { +) -> Result, Box> { let mut metrics = Registry::new(); - let configuration = C::parse_configuration(config_directory).await?; - let state = C::try_init_state(&configuration, &mut metrics).await?; + let configuration = setup.parse_configuration(config_directory).await?; + let state = setup.try_init_state(&configuration, &mut metrics).await?; Ok(ServerState::new(configuration, state, metrics)) } -pub fn create_router( - state: ServerState, - service_token_secret: Option, -) -> Router +pub fn create_router(state: ServerState, service_token_secret: Option) -> Router where + C: Connector + 'static, C::Configuration: Clone, C::State: Clone, { @@ -328,11 +345,9 @@ where )) } -pub fn create_v2_router( - state: ServerState, - service_token_secret: Option, -) -> Router +pub fn create_v2_router(state: ServerState, service_token_secret: Option) -> Router where + C: Connector + 'static, C::Configuration: Clone, C::State: Clone, { @@ -512,13 +527,16 @@ impl ndc_test::Connector for ConnectorAdapter { } } -async fn test(command: TestCommand) -> Result<(), Box> { +async fn test( + setup: Setup, + command: TestCommand, +) -> Result<(), Box> { let test_configuration = ndc_test::TestConfiguration { seed: command.seed, snapshots_dir: command.snapshots_dir, }; - let connector = make_connector_adapter::(command.configuration).await?; + let connector = make_connector_adapter(setup, command.configuration).await?; let results = ndc_test::test_connector(&test_configuration, &connector).await; if !results.failures.is_empty() { @@ -531,8 +549,11 @@ async fn test(command: TestCommand) -> Result<(), Box(command: ReplayCommand) -> Result<(), Box> { - let connector = make_connector_adapter::(command.configuration).await?; +async fn replay( + setup: Setup, + command: ReplayCommand, +) -> Result<(), Box> { + let connector = make_connector_adapter(setup, command.configuration).await?; let results = ndc_test::test_snapshots_in_directory(&connector, command.snapshots_dir).await; if !results.failures.is_empty() { @@ -545,12 +566,13 @@ async fn replay(command: ReplayCommand) -> Result<(), Box( +async fn make_connector_adapter( + setup: Setup, configuration_path: PathBuf, -) -> Result, Box> { +) -> Result, Box> { let mut metrics = Registry::new(); - let configuration = C::parse_configuration(configuration_path).await?; - let state = C::try_init_state(&configuration, &mut metrics).await?; + let configuration = setup.parse_configuration(configuration_path).await?; + let state = setup.try_init_state(&configuration, &mut metrics).await?; Ok(ConnectorAdapter { configuration, state,