jdyer1 commented on code in PR #2828:
URL: https://github.com/apache/solr/pull/2828#discussion_r1825908232
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java:
##########
@@ -173,8 +173,8 @@ public NamedList<Object> request(SolrRequest<?>
solrRequest, String collection)
"Timeout occurred while waiting response from server at: " +
pReq.url, e);
} catch (SolrException se) {
throw se;
- } catch (RuntimeException re) {
- throw new SolrServerException(re);
+ } catch (IOException | RuntimeException e) {
Review Comment:
I found that `LBHttpSolrClient` needs a `SolrServerException` to detect a
down node. Yet we still can throw an `IOException` here during
`prepareRequest`. This roughly mimics the exception handling in
`Http2SolrClient`.
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java:
##########
@@ -37,9 +37,9 @@
import org.slf4j.MDC;
/**
- * LBHttp2SolrClient or "LoadBalanced LBHttp2SolrClient" is a load balancing
wrapper around {@link
- * Http2SolrClient}. This is useful when you have multiple Solr endpoints and
requests need to be
- * Load Balanced among them.
+ * LBHttp2SolrClient or "LoadBalanced Http Solr Client" is a load balancing
wrapper around a Http
Review Comment:
I improved the wording.
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java:
##########
@@ -50,8 +52,8 @@ public void
testLBHttp2SolrClientWithTheseParamNamesInTheUrl() {
try (Http2SolrClient http2SolrClient =
new
Http2SolrClient.Builder(url).withTheseParamNamesInTheUrl(urlParamNames).build();
Review Comment:
Actually, 4/5 test methods here are using a mock client. The
`withTheseParamNamesInTheUrl` case is being tested with the clients' own unit
tests. I did add a random choice to `LBHttp2SolrClientIntegrationTest`
(renamed). I think this is pretty good coverage already.
--
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
