gortiz commented on code in PR #13930:
URL: https://github.com/apache/pinot/pull/13930#discussion_r1742031724
##########
pinot-spi/src/main/java/org/apache/pinot/spi/plugin/PluginManager.java:
##########
@@ -229,22 +257,86 @@ private void initRecordReaderClassMap() {
/**
* Loads jars recursively
* @param pluginName
- * @param directory
+ * @param directory the directory of one plugin
*/
public void load(String pluginName, File directory) {
- LOGGER.info("Trying to load plugin [{}] from location [{}]", pluginName,
directory);
- Collection<File> jarFiles = FileUtils.listFiles(directory, new
String[]{"jar"}, true);
- Collection<URL> urlList = new ArrayList<>();
- for (File jarFile : jarFiles) {
+ Path pluginPropertiesPath =
directory.toPath().resolve(PINOUT_PLUGIN_PROPERTIES_FILE_NAME);
+ if (Files.isRegularFile(pluginPropertiesPath)) {
+ Properties pluginProperties = new Properties();
+ PinotPluginConfiguration config = null;
+ try (Reader reader = Files.newBufferedReader(pluginPropertiesPath)) {
+ pluginProperties.load(reader);
+ config = new PinotPluginConfiguration(pluginProperties);
+ } catch (IOException e) {
+ LOGGER.warn("Failed to load plugin properties from {}",
pluginPropertiesPath, e);
Review Comment:
Shouldn't we just fail in this case? I can imagine a rare case where Pinot
doesn't have permissions to read the file. Would we want to load the plugin
without reading the properties? I think I would prefer to fail fast at startup
time instead.
--
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]
