dsmiley commented on a change in pull request #1675:
URL: https://github.com/apache/lucene-solr/pull/1675#discussion_r456684252
##########
File path: solr/core/src/java/org/apache/solr/core/SolrCore.java
##########
@@ -487,10 +488,13 @@ public String getName() {
}
public void setName(String v) {
+ Objects.requireNonNull(v);
+ boolean renamed = this.name != null && !this.name.equals(v);
+ assert !renamed || coreDescriptor.getCloudDescriptor() == null : "Cores
are not renamed in SolrCloud";
Review comment:
I chose to name "apiAssumeStandalone" that because it's intended for
primary service/API methods (likely coming from a user) yielding a BAD_REQUEST
instead of SERVER_ERROR. SolrCore.setName is more of an internal detail method
where an assertion seemed best. Perhaps I should add a method on SolrCore
"isSolrCloud" or "isStandalone" and/or assumeStandalone or assumeSolrCloud and
use these more pervasively? I feel that could be its own issue since there are
many places that could be enhanced/clarified to use this. Across Solr there
are various techniques like checking the subclass of the core's
SolrResourceLoader or checking if the CoreContainer has a ZkController. A
consistent approach would be nicer; perhaps separate for CoreContainer level vs
SolrCore level
----------------------------------------------------------------
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]
