dsmiley commented on a change in pull request #1547:
URL: https://github.com/apache/lucene-solr/pull/1547#discussion_r433026497
##########
File path: solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
##########
@@ -17,31 +17,25 @@
package org.apache.solr.pkg;
-import java.io.Closeable;
-import java.io.IOException;
-import java.lang.invoke.MethodHandles;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.CopyOnWriteArrayList;
-
+import org.apache.lucene.analysis.util.ResourceLoaderAware;
import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrCore;
import org.apache.solr.core.SolrResourceLoader;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.Closeable;
Review comment:
Can you please configure your IDE to put java.* import statements up
front instead of below?
##########
File path: solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
##########
@@ -301,6 +295,32 @@ public String toString() {
}
}
}
+ static class PackageResourceLoader extends SolrResourceLoader {
+
+ PackageResourceLoader(String name, List<Path> classpath, Path instanceDir,
ClassLoader parent) {
+ super(name, classpath, instanceDir, parent);
+ }
+
+ @Override
+ public <T> boolean addToCoreAware(T obj) {
+ //do not do anything
+ //this class is
Review comment:
incomplete comment? Could use an explanation as to why. SolrCoreAware
isn't handled here.
##########
File path: solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java
##########
@@ -117,6 +114,13 @@ protected void
initNewInstance(PackageLoader.Package.Version newest) {
Object instance = SolrCore.createInstance(pluginInfo.className,
pluginMeta.clazz, pluginMeta.getCleanTag(), core, newest.getLoader());
PluginBag.initInstance(instance, pluginInfo);
+ if (instance instanceof SolrCoreAware) {
Review comment:
Aha; makes sense.
##########
File path: solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
##########
@@ -301,6 +295,32 @@ public String toString() {
}
}
}
+ static class PackageResourceLoader extends SolrResourceLoader {
+
+ PackageResourceLoader(String name, List<Path> classpath, Path instanceDir,
ClassLoader parent) {
+ super(name, classpath, instanceDir, parent);
+ }
+
+ @Override
+ public <T> boolean addToCoreAware(T obj) {
+ //do not do anything
+ //this class is
+ return false;
+ }
+
+ @Override
+ public <T> boolean addToResourceLoaderAware(T obj) {
+ if (obj instanceof ResourceLoaderAware) {
+ assertAwareCompatibility(ResourceLoaderAware.class, obj);
+ try {
+ ((ResourceLoaderAware) obj).inform(this);
Review comment:
Lets figure out why the base class doesn't just immediately call inform
like this here. What test then fails? I've read an interesting JIRA issue:
https://issues.apache.org/jira/browse/SOLR-8311 (lots of info there; I
recommend reading all of it) It's been on my mind for some time now actually
since when I was cleaning up SolrResourceLoader since this "live" wart seems
like a hack.
##########
File path: solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
##########
@@ -566,25 +566,42 @@ static void clearCache() {
} catch (Exception e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
- "Error instantiating class: '" + clazz.getName() + "'", e);
+ "Error instantiating class: '" + clazz.getName() + "'", e);
}
- if (!live) {
- if (obj instanceof SolrCoreAware) {
- assertAwareCompatibility(SolrCoreAware.class, obj);
- waitingForCore.add((SolrCoreAware) obj);
+ addToCoreAware(obj);
+ addToResourceLoaderAware(obj);
+ if(!live) {
Review comment:
Should this not be in its own method now too?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
