jdyer1 commented on code in PR #3730:
URL: https://github.com/apache/solr/pull/3730#discussion_r2407051587
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClientBuilderBase.java:
##########
@@ -47,6 +47,37 @@ public abstract class HttpSolrClientBuilderBase<
public abstract C build();
+ /**
+ * Provide a seed HttpSolrClient for the builder values, values can still be
overridden by using
+ * builder methods
Review Comment:
I would have liked to make this a constructor-only option, but this is not
new code and not a new doc-comment. I merely moved this for this PR/ticket,
and would like to remain as focused as possible.
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2ClusterStateProvider.java:
##########
@@ -19,33 +19,34 @@
import java.io.IOException;
import java.util.List;
-import org.apache.solr.client.solrj.SolrClient;
-public class Http2ClusterStateProvider extends BaseHttpClusterStateProvider {
- final Http2SolrClient httpClient;
- final boolean closeClient;
+public class Http2ClusterStateProvider<C extends HttpSolrClientBase>
+ extends BaseHttpClusterStateProvider {
+ final C httpClient;
- public Http2ClusterStateProvider(List<String> solrUrls, Http2SolrClient
httpClient)
- throws Exception {
- this.httpClient = httpClient == null ? new
Http2SolrClient.Builder().build() : httpClient;
- this.closeClient = httpClient == null;
+ public Http2ClusterStateProvider(List<String> solrUrls, C httpClient) throws
Exception {
+ if (httpClient == null) {
Review Comment:
I know some JDK code throws NPE in this situation, but it seems to me
`IllegalArgumentException` is the better choice
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2ClusterStateProvider.java:
##########
@@ -19,33 +19,34 @@
import java.io.IOException;
import java.util.List;
-import org.apache.solr.client.solrj.SolrClient;
-public class Http2ClusterStateProvider extends BaseHttpClusterStateProvider {
- final Http2SolrClient httpClient;
- final boolean closeClient;
+public class Http2ClusterStateProvider<C extends HttpSolrClientBase>
+ extends BaseHttpClusterStateProvider {
+ final C httpClient;
- public Http2ClusterStateProvider(List<String> solrUrls, Http2SolrClient
httpClient)
- throws Exception {
- this.httpClient = httpClient == null ? new
Http2SolrClient.Builder().build() : httpClient;
- this.closeClient = httpClient == null;
+ public Http2ClusterStateProvider(List<String> solrUrls, C httpClient) throws
Exception {
Review Comment:
Thank you for pointing this out. I am changing this to not close the client
as this is what we did before in the case one was passed-in (indeed, our code
always passes one in, so this would have been a regression I think).
--
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]
