-
Notifications
You must be signed in to change notification settings - Fork 19
feat!: Update StreamingDataSourceBuilder to require HttpTransport #131
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
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 |
|---|---|---|
|
|
@@ -24,7 +24,7 @@ jobs: | |
| - name: Setup rust tooling | ||
| if: ${{ steps.release.outputs['launchdarkly-server-sdk--release_created'] == 'true' }} | ||
| run: | | ||
| rustup override set 1.83 | ||
| rustup override set 1.85 | ||
| rustup component add rustfmt clippy | ||
|
|
||
| - uses: launchdarkly/gh-actions/actions/[email protected] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| use eventsource_client as es; | ||
| use futures::future::FutureExt; | ||
| use launchdarkly_server_sdk::{ | ||
| Context, ContextBuilder, MigratorBuilder, MultiContextBuilder, Reference, | ||
|
|
@@ -20,6 +21,7 @@ use crate::command_params::{ | |
| MigrationOperationResponse, MigrationVariationResponse, SecureModeHashResponse, | ||
| }; | ||
| use crate::HttpsConnector; | ||
| use crate::StreamingHttpsConnector; | ||
| use crate::{ | ||
| command_params::{ | ||
| CommandParams, CommandResponse, EvaluateAllFlagsParams, EvaluateAllFlagsResponse, | ||
|
|
@@ -36,6 +38,7 @@ impl ClientEntity { | |
| pub async fn new( | ||
| create_instance_params: CreateInstanceParams, | ||
| connector: HttpsConnector, | ||
| streaming_https_connector: StreamingHttpsConnector, | ||
|
Member
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. The event source and the SDK both relied on hyper pre-1.0. Since the SDK still relies on it, we have a couple of different types we have to juggle. As a result, we have our connector, and then our streaming specific connector. I plan on updating the hyper usage for the rest of the library next, which should remove this silliness. But it's needed for now. |
||
| ) -> Result<Self, BuildError> { | ||
| let mut config_builder = | ||
| ConfigBuilder::new(&create_instance_params.configuration.credential); | ||
|
|
@@ -71,6 +74,8 @@ impl ClientEntity { | |
| } | ||
|
|
||
| if let Some(streaming) = create_instance_params.configuration.streaming { | ||
| let transport = | ||
| es::HyperTransport::builder().build_with_connector(streaming_https_connector); | ||
| if let Some(base_uri) = streaming.base_uri { | ||
| service_endpoints_builder.streaming_base_url(&base_uri); | ||
| } | ||
|
|
@@ -79,7 +84,7 @@ impl ClientEntity { | |
| if let Some(delay) = streaming.initial_retry_delay_ms { | ||
| streaming_builder.initial_reconnect_delay(Duration::from_millis(delay)); | ||
| } | ||
| streaming_builder.https_connector(connector.clone()); | ||
| streaming_builder.transport(transport); | ||
|
|
||
| config_builder = config_builder.data_source(&streaming_builder); | ||
| } else if let Some(polling) = create_instance_params.configuration.polling { | ||
|
|
@@ -98,8 +103,10 @@ impl ClientEntity { | |
| // If we didn't specify streaming or polling, we fall back to basic streaming. The only | ||
| // customization we provide is the https connector to support testing multiple | ||
| // connectors. | ||
| let transport = | ||
| es::HyperTransport::builder().build_with_connector(streaming_https_connector); | ||
| let mut streaming_builder = StreamingDataSourceBuilder::new(); | ||
| streaming_builder.https_connector(connector.clone()); | ||
| streaming_builder.transport(transport); | ||
| config_builder = config_builder.data_source(&streaming_builder); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -132,6 +132,7 @@ async fn create_client( | |
| let client_entity = match ClientEntity::new( | ||
| create_instance_params.into_inner(), | ||
| app_state.https_connector.clone(), | ||
| app_state.streaming_https_connector.clone(), | ||
| ) | ||
| .await | ||
| { | ||
|
|
@@ -206,12 +207,19 @@ struct AppState { | |
| counter: Mutex<u32>, | ||
| client_entities: Mutex<HashMap<u32, ClientEntity>>, | ||
| https_connector: HttpsConnector, | ||
| streaming_https_connector: StreamingHttpsConnector, | ||
| } | ||
|
|
||
| #[cfg(feature = "rustls")] | ||
| type HttpsConnector = hyper_rustls::HttpsConnector<HttpConnector>; | ||
| #[cfg(feature = "rustls")] | ||
| type StreamingHttpsConnector = hyper_util::client::legacy::connect::HttpConnector; | ||
|
|
||
| #[cfg(feature = "tls")] | ||
| type HttpsConnector = hyper_tls::HttpsConnector<HttpConnector>; | ||
| #[cfg(feature = "tls")] | ||
| type StreamingHttpsConnector = | ||
| hyper1_tls::HttpsConnector<hyper_util::client::legacy::connect::HttpConnector>; | ||
|
Member
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. Note the |
||
|
|
||
| #[actix_web::main] | ||
| async fn main() -> std::io::Result<()> { | ||
|
|
@@ -228,6 +236,8 @@ async fn main() -> std::io::Result<()> { | |
|
|
||
| let (tx, rx) = mpsc::channel::<()>(); | ||
|
|
||
| #[cfg(feature = "rustls")] | ||
| let streaming_https_connector = hyper_util::client::legacy::connect::HttpConnector::new(); | ||
| #[cfg(feature = "rustls")] | ||
| let connector = hyper_rustls::HttpsConnectorBuilder::new() | ||
| .with_native_roots() | ||
|
|
@@ -236,13 +246,16 @@ async fn main() -> std::io::Result<()> { | |
| .enable_http2() | ||
| .build(); | ||
|
|
||
| #[cfg(feature = "tls")] | ||
| let streaming_https_connector = hyper1_tls::HttpsConnector::new(); | ||
| #[cfg(feature = "tls")] | ||
| let connector = hyper_tls::HttpsConnector::new(); | ||
|
|
||
| let state = web::Data::new(AppState { | ||
| counter: Mutex::new(0), | ||
| client_entities: Mutex::new(HashMap::new()), | ||
| https_connector: connector, | ||
| streaming_https_connector: streaming_https_connector, | ||
| }); | ||
|
|
||
| let server = HttpServer::new(move || { | ||
|
|
||
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.
@kinyoklion turns out you can specify which version you want based on a feature, and this is me doing it. 😄