launchdarkly / rust-server-sdk

LaunchDarkly Server-Side SDK for Rust
https://docs.launchdarkly.com/sdk/server-side/rust
Other
18 stars 13 forks source link

add support for arbitrary connectors to StreamingDataSourceBuilder #16

Closed cab closed 10 months ago

cab commented 2 years ago

Requirements

Describe the solution you've provided

This PR adds support for using an arbitrary hyper connector with StreamingDataSourceBuilder. This is needed in order to use this crate with a unix socket, i.e. through hyperlocal.

Describe alternatives you've considered

This is a breaking change due to the new generic type parameter on StreamingDataSourceBuilder. To avoid this, a new struct implementing DataSourceFactory could be added (e.g. StreamingDataSourceBuilderWithConnector).

Alternatively, the DataSourceFactory trait (and ideally the StreamingDataSource struct) could be made part of the public API, allowing a custom connector to be used with an implementation of that trait.

cab commented 2 years ago

The failing tests in CircleCI seem to be failing for main as well.

keelerm84 commented 2 years ago

Hello @cab. Thank you for your contribution. I haven't had a chance to review your PR yet. I will try to do that by early next week. Just wanted you to know I've seen it and it is on my radar to address!

keelerm84 commented 2 years ago

Thanks @cab for your patience while I circled back to this.

A question for you: this modification adds unix socket support through the streaming connection. Do you have a use case where you would also benefit from this for sending events or potentially the polling mechanism?

While not yet widely supported across our SDKs, unix socket support is something we do support in Java, Go, and Ruby. We strive to keep implementations as architecturally consistent as possible; I think the implementation for this feature might need to be expanded.

I am happy to take over the development of this feature as I think there are still some unanswered questions we have about scope. Does that sound okay to you?

cab commented 2 years ago

Personally I don't need the other functionality, but I agree with you that consistency there is a good idea.

Definitely happy to have you take over development. Thanks for looking into this! Please let me know if I can help in any way.

Do you have a rough idea of when this might be prioritized or released?

keelerm84 commented 2 years ago

My hope is to look at tackling this over the next week. Of course I cannot guarantee that, but that's my general plan. I will keep you updated if that timeline changes.

cab commented 2 years ago

Sounds good to me, thank you!

keelerm84 commented 1 year ago

Hey @cab just checking on this task. Some initial work on this has begun. There are some competing priorities that are slowing down development a bit, but we are still working on moving this forward for you. Will keep you posted as it progresses. Thanks for your continued patience on this!

keelerm84 commented 10 months ago

Hey @cab, I just released v2 of this SDK which should allow you to provide different connectors now for streaming, polling, or event processing.

I am sorry I wasn't able to merge your work in directly as part of this overhaul. However, please know I relied heavily on your work and I really appreciate the time and insight you have provided on this task.

Please let me know if you have any questions or concerns about the new release.