jdyer1 commented on code in PR #2899:
URL: https://github.com/apache/solr/pull/2899#discussion_r1889342889
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java:
##########
@@ -404,23 +400,6 @@ public Builder withCollectionCacheTtl(long timeToLive,
TimeUnit unit) {
return this;
}
- /**
- * Set the internal http client.
- *
- * <p>Note: closing the httpClient instance is at the responsibility of
the caller.
- *
- * @param httpClient http client
- * @return this
- */
- public Builder withHttpClient(Http2SolrClient httpClient) {
Review Comment:
This is the most controversial part of this PR, and I do not want to merge a
potentially disruptive API change like this without buy-in. I do not think
this is much of a burden though, because the Builder has
`withInternalClientBuilder`, which users should be able to easily migrate to
use.
The purpose of forcing this is that `CloudHttp2SolrClient` in turn passes
this Client(Builder) to `LBHttp2SolrClient`. But as noted before,
`LBHttp2SolrClient` doesn't really use this Client either; it clones it using
`Http2SolrClient.builder#withHttpClient`. Yet we've made `LBHttp2SolrClient`
generic so this means that any subclass of `HttpSolrClientBase` needs to also
have a way to be cloned. Personally I think clone methods like this are
brittle and a maintenance burden, and I don't want to need to write one for
`HttpJdkSolrClient`. It also seems like a better API: by telling the user we
want a Builder, we signal that we will create Clients as needed. Passing in an
already-built Client signals that we will use that particular one, which is not
always the case here.
--
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]
