paul-rogers commented on code in PR #12368:
URL: https://github.com/apache/druid/pull/12368#discussion_r867224761
##########
server/src/main/java/org/apache/druid/curator/CuratorModule.java:
##########
@@ -48,35 +48,22 @@
public class CuratorModule implements Module
{
- static final String CURATOR_CONFIG_PREFIX = "druid.zk.service";
-
- static final String EXHIBITOR_CONFIG_PREFIX = "druid.exhibitor.service";
+ private static final Logger log = new Logger(CuratorModule.class);
private static final int BASE_SLEEP_TIME_MS = 1000;
-
private static final int MAX_SLEEP_TIME_MS = 45000;
-
private static final int MAX_RETRIES = 29;
- private static final Logger log = new Logger(CuratorModule.class);
-
@Override
public void configure(Binder binder)
{
- JsonConfigProvider.bind(binder, CURATOR_CONFIG_PREFIX,
ZkEnablementConfig.class);
- JsonConfigProvider.bind(binder, CURATOR_CONFIG_PREFIX,
CuratorConfig.class);
- JsonConfigProvider.bind(binder, EXHIBITOR_CONFIG_PREFIX,
ExhibitorConfig.class);
+ JsonConfigProvider.bind(binder, CuratorConfig.CONFIG_PREFIX,
ZkEnablementConfig.class);
+ JsonConfigProvider.bind(binder, CuratorConfig.CONFIG_PREFIX,
CuratorConfig.class);
+ JsonConfigProvider.bind(binder, ExhibitorConfig.CONFIG_PREFIX,
ExhibitorConfig.class);
}
- @Provides
- @LazySingleton
- @SuppressForbidden(reason = "System#err")
- public CuratorFramework makeCurator(ZkEnablementConfig zkEnablementConfig,
CuratorConfig config, EnsembleProvider ensembleProvider, Lifecycle lifecycle)
+ public static CuratorFramework createCurator(CuratorConfig config,
EnsembleProvider ensembleProvider)
Review Comment:
This is a tricky one. The original code creates a curator framework via a
Guice provider. We have to keep that as that's what Druid services require.
The test code wants to use ZK, via curator, but without Guice, since Guice
adds extra complexity. It turns out that what we want is to use the builder
from the two config items. Rather than copy/paste that code, this refactoring
makes it available outside of Guice.
The new static method have to be public so tests can reach them. I believe
that the existing instance methods have to also be public so Guice can call
them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
