mlbiscoc commented on code in PR #2935:
URL: https://github.com/apache/solr/pull/2935#discussion_r1925380385
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java:
##########
@@ -387,6 +398,19 @@ public String getPolicyNameByCollection(String coll) {
+ "ZkClientClusterStateProvider can be used for this."); // TODO
}
+ private List<URL> setConfiguredNodes(List<String> solrUrls) {
Review Comment:
You're right bad naming. It's only used in the `init` so Instead I just
removed the function as I don't think we need another utility method and it was
somewhat small enough inlined the whole thing into the stream where it was
called. Can move it into a private static utility if you prefer otherwise.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/ClusterStateProviderTest.java:
##########
@@ -204,8 +204,8 @@ public void
testClusterStateProviderDownedInitialLiveNodes() throws Exception {
var jettyNode1 = cluster.getJettySolrRunner(0);
var jettyNode2 = cluster.getJettySolrRunner(1);
- String nodeName1 =
getNodeNameFromSolrUrl(jettyNode1.getBaseUrl().toString());
- String nodeName2 =
getNodeNameFromSolrUrl(jettyNode2.getBaseUrl().toString());
+ String nodeName1 =
getNodeNameForBaseUrl(jettyNode1.getBaseUrl().toString());
Review Comment:
Yeah it was deliberate. Was mostly to keep consistency between the names.
`getNodeNameForBaseUrl` and `getBaseUrlForNodeName` but can switch it back if
you prefer.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
