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: issues-unsubscr...@solr.apache.org 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