m1a2st commented on code in PR #19397:
URL: https://github.com/apache/kafka/pull/19397#discussion_r2033040494
##########
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java:
##########
@@ -620,21 +621,22 @@ private Map<String, ConfigProvider>
instantiateConfigProviders(
}
}
// Instantiate Config Providers
- Map<String, ConfigProvider> configProviderInstances = new HashMap<>();
+ Map<String, Plugin<ConfigProvider>> configProviderPluginInstances =
new HashMap<>();
for (Map.Entry<String, String> entry : providerMap.entrySet()) {
try {
String prefix = CONFIG_PROVIDERS_CONFIG + "." + entry.getKey()
+ CONFIG_PROVIDERS_PARAM;
Map<String, ?> configProperties =
configProviderProperties(prefix, providerConfigProperties);
ConfigProvider provider = Utils.newInstance(entry.getValue(),
ConfigProvider.class);
provider.configure(configProperties);
- configProviderInstances.put(entry.getKey(), provider);
+ Plugin<ConfigProvider> providerPlugin =
Plugin.wrapInstance(provider, null, CONFIG_PROVIDERS_CONFIG);
Review Comment:
I don't think we need to wrap `provider` into a `Plugin` at this point,
since the metrics are `null` and it doesn't seem to serve any practical purpose.
##########
connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorMakerConfig.java:
##########
@@ -269,18 +270,19 @@ List<String> configProviders() {
Map<String, String> transform(Map<String, String> props) {
// transform worker config according to config.providers
List<String> providerNames = configProviders();
- Map<String, ConfigProvider> providers = new HashMap<>();
+ Map<String, Plugin<ConfigProvider>> providerPlugins = new HashMap<>();
for (String name : providerNames) {
- ConfigProvider configProvider = plugins.newConfigProvider(
+ Plugin<ConfigProvider> configProviderPlugin =
plugins.newConfigProvider(
this,
- CONFIG_PROVIDERS_CONFIG + "." + name,
- Plugins.ClassLoaderUsage.PLUGINS
+ name,
+ Plugins.ClassLoaderUsage.PLUGINS,
+ null
Review Comment:
ditto: I don't think we need to wrap provider into a Plugin at this point,
since the metrics are null and it doesn't seem to serve any practical purpose.
##########
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java:
##########
@@ -587,14 +588,14 @@ private Map<String, Object>
configProviderProperties(String configProviderPrefix
* config.providers : A comma-separated list of names for providers.
* config.providers.{name}.class : The Java class name for a provider.
* config.providers.{name}.param.{param-name} : A parameter to be passed
to the above Java class on initialization.
- * returns a map of config provider name and its instance.
+ * returns a map of config provider name and its instance wrapped in a
{@link org.apache.kafka.common.internals.Plugin}.
*
* @param indirectConfigs The map of potential variable configs
* @param providerConfigProperties The map of config provider configs
* @param classNameFilter Filter for config provider class names
- * @return map of config provider name and its instance.
+ * @return map of config provider name and its instance wrapped in a
{@link org.apache.kafka.common.internals.Plugin}.
Review Comment:
We could keep the original document, FYI
https://github.com/apache/kafka/pull/19050#discussion_r2031382775
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
