dsmiley commented on a change in pull request #135:
URL: https://github.com/apache/solr/pull/135#discussion_r641221511
##########
File path:
solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
##########
@@ -62,7 +62,12 @@ public String configSetName(CoreDescriptor cd) {
@Override
public boolean checkConfigExists(String configName) throws IOException {
Path configSetDirectory = configSetBase.resolve(configName);
- return Files.isDirectory(configSetDirectory);
+ if (Files.isDirectory(configSetDirectory)) {
Review comment:
Why the isDirectory check; why not just check for solrconfig.xml?
##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
##########
@@ -151,6 +151,19 @@ public boolean checkConfigExists(String configName) throws
IOException {
}
}
+ @Override
+ public boolean checkConfigExistsWithSolrConfigXml(String configName) throws
IOException {
+ try {
+ Boolean existsConfig = zkClient.exists(CONFIGS_ZKNODE + "/" +
configName, true);
+ if (existsConfig == null || existsConfig.booleanValue() == false) return
false;
+ Boolean existsSolrConfigXml = zkClient.exists(CONFIGS_ZKNODE + "/" +
configName + "/solrconfig.xml", true);
+ return existsSolrConfigXml;
Review comment:
I'm missing something; why two calls to ZK just to check that a config
exists? If solrconfig.xml is now necessary, then why check the parent?
--
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
