Skip to content
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

Refactor Druid HTTP client to support extending a base Druid client #622

Conversation

jtuglu-netflix
Copy link
Contributor

@jtuglu-netflix jtuglu-netflix commented Dec 3, 2024

Overview

This allows for extensibility of Druid Connector client (through inheritance). Implementing a TLS-supported (or whatever signing mechanism you have) client is now much easier as users can apply the necessary overrides of the RestTemplate after Base ctor returns.

Changes

  • Change Coordinator URI to Router URI, as this is how Druid clusters are accessed now.
  • Create BaseDruidHttpClientConfig which is an abstract class that specifies the Bean(s) to implement for any MetcatDruidClient.
  • Modify DruidConnectorFactory to take in any class which extends the BaseDruidHttpClientConfig class.

Notes:

  • Idea here is that to extend the default Druid connector, you can create your own *Plugin.java file, and pass the custom MetacatDruidClient client implementation class type into the DruidConnectorFactory ctor.
  • Having an abstract class instead of a BaseDruidHttpClientConfig with default impls is regrettable, but I was getting duplicate Bean name warnings when instantiating multiple instances of the interface.

@jtuglu-netflix jtuglu-netflix force-pushed the refactor-druid-connector-http-client-to-support-extending-a-base-druid-client branch 4 times, most recently from 4071bf0 to 0f5318e Compare December 5, 2024 18:42
@jtuglu-netflix jtuglu-netflix force-pushed the refactor-druid-connector-http-client-to-support-extending-a-base-druid-client branch from 0f5318e to 0e77004 Compare December 6, 2024 19:17
@stevie9868 stevie9868 merged commit d579ca9 into master Dec 6, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants