snehashisp commented on code in PR #16984:
URL: https://github.com/apache/kafka/pull/16984#discussion_r1870985286


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java:
##########
@@ -122,11 +196,54 @@ protected Class<?> loadClass(String name, boolean 
resolve) throws ClassNotFoundE
         return super.loadClass(fullName, resolve);
     }
 
+    protected Class<?> loadVersionedPluginClass(
+        String name,
+        VersionRange range,
+        boolean resolve
+    ) throws VersionedPluginLoadingException, ClassNotFoundException {
+
+        String fullName = aliases.getOrDefault(name, name);
+        PluginClassLoader pluginLoader = pluginClassLoader(fullName, range);
+        Class<?> plugin;
+        if (pluginLoader != null) {
+            log.trace("Retrieving loaded class '{}' from '{}'", name, 
pluginLoader);
+            plugin = pluginLoader.loadClass(fullName, resolve);
+        } else {
+            plugin = super.loadClass(fullName, resolve);
+            // if we are loading a plugin class from the parent classloader, 
we need to check if the version
+            // matches the range
+            String pluginVersion;
+            try (LoaderSwap classLoader = 
PluginScanner.withClassLoader(plugin.getClassLoader())) {
+                pluginVersion = 
PluginScanner.versionFor(plugin.getDeclaredConstructor().newInstance());
+            } catch (ReflectiveOperationException | LinkageError e) {
+                throw new VersionedPluginLoadingException(String.format(
+                        "Plugin %s was loaded with %s but failed to determine 
its version",
+                        name,
+                        plugin.getClassLoader()
+                ), e);
+            }

Review Comment:
   Is this part not really required? IIUC the plugins present in classpath are 
also scanned and part of the `pluginLoaders` map, and it should be found with 
the `pluginClassLoader` logic. `super.loadClass` should always throw a 
`ClassNotFoundException`. Otherwise, if there are edge cases where 
super.loadClass does find the class (maybe the plugin is part of the Bootstrap 
loader) then instantiation the class here is the only way to confirm version 
requirements are addressed. 



-- 
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

Reply via email to