[GitHub] [lucene-solr] dsmiley commented on a change in pull request #2343: SOLR-15100: make ConfigSetService configurable in solr.xml

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

dsmiley commented on a change in pull request #2343:
URL: https://github.com/apache/lucene-solr/pull/2343#discussion_r582174260



##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -2027,6 +2027,10 @@ public ConfigSetsHandler getConfigSetsHandler() {
     return configSetsHandler;
   }
 
+  public ConfigSetService getCoreConfigService() {

Review comment:
       Lets call this getConfigSetService and maybe rename the field now (or later)

##########
File path: solr/core/src/java/org/apache/solr/core/NodeConfig.java
##########
@@ -89,6 +89,8 @@
   private final boolean fromZookeeper;
   private final String defaultZkHost;
 
+  private final String configSetServiceClass;

Review comment:
       Move up to the other classes to be more organized

##########
File path: solr/core/src/java/org/apache/solr/core/ConfigSetService.java
##########
@@ -56,11 +58,24 @@
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   public static ConfigSetService createConfigSetService(NodeConfig nodeConfig, SolrResourceLoader loader, ZkController zkController) {
-    if (zkController == null) {
+
+    String configSetServiceClass = nodeConfig.getConfigSetServiceClass();
+
+    if(configSetServiceClass != null){
+      try {
+        Class<? extends ConfigSetService> clazz = loader.findClass(configSetServiceClass, ConfigSetService.class);
+        Constructor<? extends ConfigSetService> constructor

Review comment:
       I would rather we pass the NodeConfig instead of the boolean and let the implementation examine whatever it may want.  I think that's easier to maintain.




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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org