gortiz commented on code in PR #13930: URL: https://github.com/apache/pinot/pull/13930#discussion_r1742062595
########## 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); + } + + final ClassLoader baseClassLoader = ClassLoader.getPlatformClassLoader(); + + Collection<URL> urlList; + try (Stream<Path> pluginClasspathEntries = Files.list(directory.toPath())) { + urlList = pluginClasspathEntries.map(p -> { + try { + return p.toUri().toURL(); + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } + }).collect(Collectors.toList()); + } catch (IOException e) { + throw new RuntimeException(e); + } + try { - urlList.add(jarFile.toURI().toURL()); - } catch (MalformedURLException e) { - LOGGER.error("Unable to load plugin [{}] jar file [{}]", pluginName, jarFile, e); + ClassRealm pluginRealm = _classWorld.newRealm( + pluginName, + baseClassLoader); + urlList.forEach(pluginRealm::addURL); + + ClassRealm pinotRealm = _classWorld.getClassRealm(PINOT_REALMID); + + // All packages to look up in pinot realm BEFORE itself + Stream<String> importedPinotPackages = + Stream.of("org.apache.pinot.spi"); // this works like a prefix, so ALL spi classes will be accessible + importedPinotPackages.forEach(p -> pluginRealm.importFrom(pinotRealm, p)); + + // Additional importForm as specified by the plugin configuration + if (config != null) { + config.getImportsFromPerRealm().forEach((r, ifs) -> { + try { + ClassRealm cr = _classWorld.getRealm(r); + ifs.forEach(i -> pluginRealm.importFrom(cr, i)); + } catch (NoSuchRealmException e) { + LOGGER.warn("{} realm does not exist", r); + } + } + ); + + // Important: parent is not the same as baseclassloader (see pluginRealm above) + // baseClassLoader is BEFORE self classloader (should be Platform class loader) + // parentClassLoader is AFTER self classloader + config.getParentRealmId().map(_classWorld::getClassRealm).ifPresent(pluginRealm::setParentRealm); Review Comment: My bad, it is registered in the _classWorld -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org