asafm commented on code in PR #19208:
URL: https://github.com/apache/pulsar/pull/19208#discussion_r1081486335
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/MetadataStoreFactoryImpl.java:
##########
@@ -45,19 +62,50 @@ public static MetadataStoreExtended createExtended(String
metadataURL, MetadataS
private static MetadataStore newInstance(String metadataURL,
MetadataStoreConfig metadataStoreConfig,
boolean enableSessionWatcher)
throws MetadataStoreException {
+ MetadataStoreProvider provider = findProvider(metadataURL);
+ if (provider != null) {
+ return provider.create(metadataURL, metadataStoreConfig,
enableSessionWatcher);
+ }
+ return new ZKMetadataStore(metadataURL, metadataStoreConfig,
enableSessionWatcher);
+ }
+
+ static void loadProviders() {
+ String factoryClasses =
System.getProperty(METADATASTORE_PROVIDERS_PROPERTY);
Review Comment:
1. @andrasbeni The only tool I see here is CompactorTool, the rest are just
plain Pulsar services that have access to Pulsar config. The Comparator tool
itself has an argument `@Parameter(names = {"-c", "--broker-conf"}, description
= "Configuration file for Broker")` so it also has access to the configuration
file.
So based on that, it seems that we can use the Pulsar configuration for
those provider classes, no?
2. That one is bit odd. If Pulsar is the one creating the BK client, why not
provide the Client Metadata driver through the BK client it self. Why go
through a System property to statically initialize? It may make sense inside BK
it self. Maybe @hangc0276 knows. We can also call it when Pulsar initializes as
it has method for that.
Based on (1) and (2) I don't understand why we can't use Pulsar
configuration for that config @andrasbeni
--
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]
