[GitHub] [drill] arina-ielchiieva commented on a change in pull request #1780: DRILL-7030: Make format plugins fully pluggable

[GitBox <gi...@apache.org>]

arina-ielchiieva commented on a change in pull request #1780: DRILL-7030: Make format plugins fully pluggable
URL: https://github.com/apache/drill/pull/1780#discussion_r280834454
 
 

 ##########
 File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
 ##########
 @@ -334,6 +331,48 @@ private StoragePlugins loadBootstrapPlugins(LogicalPlanPersistence lpPersistence
     }
   }
 
+  private void loadStoragePlugins(URL url, StoragePlugins bootstrapPlugins, Map<String, URL> pluginURLMap, LogicalPlanPersistence lpPersistence) throws IOException {
+    StoragePlugins plugins = getPluginsFromResource(url, lpPersistence);
+    plugins.forEach(plugin -> {
+      StoragePluginConfig oldPluginConfig = bootstrapPlugins.putIfAbsent(plugin.getKey(), plugin.getValue());
+      if (oldPluginConfig != null) {
+        logger.warn("Duplicate plugin instance '{}' defined in [{}, {}], ignoring the later one.",
+            plugin.getKey(), pluginURLMap.get(plugin.getKey()), url);
+      } else {
+        pluginURLMap.put(plugin.getKey(), url);
+      }
+    });
+  }
+
+  private void loadFormatPlugins(URL url, StoragePlugins bootstrapPlugins, Map<String, URL> pluginURLMap, LogicalPlanPersistence lpPersistence) throws IOException {
+    StoragePlugins plugins = getPluginsFromResource(url, lpPersistence);
+    plugins.forEach(formatPlugin -> {
+      String targetStoragePluginName = formatPlugin.getKey();
+      StoragePluginConfig storagePlugin = bootstrapPlugins.getConfig(targetStoragePluginName);
+      StoragePluginConfig formatPluginValue = formatPlugin.getValue();
+      if (storagePlugin == null) {
+        logger.warn("No storage plugins with the given name are registered: '{}'", targetStoragePluginName);
+      } else if (storagePlugin instanceof FileSystemConfig && formatPluginValue instanceof FileSystemConfig) {
+        FileSystemConfig targetPlugin = (FileSystemConfig) storagePlugin;
+        ((FileSystemConfig) formatPluginValue).getFormats().forEach((formatName, formatValue) -> {
+          if (targetPlugin.getFormats().containsKey(formatName)) {
 
 Review comment:
   Use putIfAbsent as in `loadStoragePlugins`

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services