-
Notifications
You must be signed in to change notification settings - Fork 141
connectors: add transport endpoint registries #6522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,14 @@ use anyhow::{Error as AnyError, Result as AnyResult}; | |
| use chrono::{DateTime, Utc}; | ||
| use dyn_clone::DynClone; | ||
| use feldera_types::adapter_stats::ConnectorHealth; | ||
| use feldera_types::config::FtModel; | ||
| use feldera_types::config::{FtModel, TransportConfig}; | ||
| use feldera_types::coordination::Completion; | ||
| use feldera_types::program_schema::Relation; | ||
| use rmpv::{Value as RmpValue, ext::Error as RmpDecodeError}; | ||
| use serde::Deserialize; | ||
| use serde::de::DeserializeOwned; | ||
| use serde_json::Value as JsonValue; | ||
| use std::collections::VecDeque; | ||
| use std::collections::{BTreeMap, VecDeque}; | ||
| use std::fmt::Display; | ||
| use std::marker::PhantomData; | ||
| use std::sync::atomic::{AtomicBool, Ordering}; | ||
|
|
@@ -75,6 +75,47 @@ pub trait TransportInputEndpoint: InputEndpoint { | |
| ) -> AnyResult<Box<dyn InputReader>>; | ||
| } | ||
|
|
||
| /// Factory for creating input transport endpoints from transport configuration. | ||
| pub trait InputTransportEndpointFactory: Send + Sync { | ||
| fn create(&self, config: &TransportConfig) -> AnyResult<Box<dyn TransportInputEndpoint>>; | ||
| } | ||
|
|
||
| /// Registry of input transport endpoint factories keyed by transport name. | ||
| #[derive(Default)] | ||
| pub struct InputTransportRegistry { | ||
| registered: BTreeMap<String, Arc<dyn InputTransportEndpointFactory>>, | ||
| } | ||
|
|
||
| impl InputTransportRegistry { | ||
| pub fn new() -> Self { | ||
| Self { | ||
| registered: BTreeMap::new(), | ||
| } | ||
| } | ||
|
|
||
| pub fn register( | ||
| &mut self, | ||
| name: impl Into<String>, | ||
| factory: Box<dyn InputTransportEndpointFactory>, | ||
| ) { | ||
| self.registered.insert(name.into(), Arc::from(factory)); | ||
| } | ||
|
|
||
| pub fn get(&self, name: &str) -> Option<Arc<dyn InputTransportEndpointFactory>> { | ||
| self.registered.get(name).cloned() | ||
| } | ||
|
|
||
| pub fn create_endpoint( | ||
| &self, | ||
| config: &TransportConfig, | ||
| ) -> AnyResult<Option<Box<dyn TransportInputEndpoint>>> { | ||
| let Some(factory) = self.get(config.name().as_str()) else { | ||
| return Ok(None); | ||
| }; | ||
| factory.create(config).map(Some) | ||
| } | ||
| } | ||
|
|
||
| #[doc(hidden)] | ||
| pub trait IntegratedInputEndpoint: InputEndpoint { | ||
| fn open( | ||
|
|
@@ -1094,6 +1135,56 @@ pub trait OutputEndpoint: Send { | |
| } | ||
| } | ||
|
|
||
| /// Factory for creating output transport endpoints from transport configuration. | ||
| pub trait OutputTransportEndpointFactory: Send + Sync { | ||
| fn create( | ||
| &self, | ||
| config: &TransportConfig, | ||
| endpoint_name: &str, | ||
| fault_tolerant: bool, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this take the enum or just a boolean? |
||
| ) -> AnyResult<Box<dyn OutputEndpoint>>; | ||
| } | ||
|
|
||
| /// Registry of output transport endpoint factories keyed by transport name. | ||
| #[derive(Default)] | ||
| pub struct OutputTransportRegistry { | ||
| registered: BTreeMap<String, Arc<dyn OutputTransportEndpointFactory>>, | ||
| } | ||
|
|
||
| impl OutputTransportRegistry { | ||
| pub fn new() -> Self { | ||
| Self { | ||
| registered: BTreeMap::new(), | ||
| } | ||
| } | ||
|
|
||
| pub fn register( | ||
| &mut self, | ||
| name: impl Into<String>, | ||
| factory: Box<dyn OutputTransportEndpointFactory>, | ||
| ) { | ||
| self.registered.insert(name.into(), Arc::from(factory)); | ||
| } | ||
|
|
||
| pub fn get(&self, name: &str) -> Option<Arc<dyn OutputTransportEndpointFactory>> { | ||
| self.registered.get(name).cloned() | ||
| } | ||
|
|
||
| pub fn create_endpoint( | ||
| &self, | ||
| config: &TransportConfig, | ||
| endpoint_name: &str, | ||
| fault_tolerant: bool, | ||
| ) -> AnyResult<Option<Box<dyn OutputEndpoint>>> { | ||
| let Some(factory) = self.get(config.name().as_str()) else { | ||
| return Ok(None); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same fix on the output side. the low-level registry can still say none for fallback, but the path users hit now gives an actual error when a registry-backed output transport has no factory. lgtm to me this way. |
||
| }; | ||
| factory | ||
| .create(config, endpoint_name, fault_tolerant) | ||
| .map(Some) | ||
| } | ||
| } | ||
|
|
||
| /// An [UnboundedReceiver] wrapper for [InputReaderCommand] for fault-tolerant connectors. | ||
| /// | ||
| /// A fault-tolerant connector wants to receive, in order: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should return some error to help users diagnose why the registry cannot be found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, handled in 3f43c20. i kept the raw registry returning none when the key is missing bc imo we still need that for the integrated connector fallback. the controller/helper path checks if the transport is one of the registry ones, and only then returns the missing-factory error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be documented