dsmiley commented on code in PR #3881:
URL: https://github.com/apache/solr/pull/3881#discussion_r2544103363
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java:
##########
@@ -165,8 +165,9 @@ public CompletableFuture<NamedList<Object>> requestAsync(
}
}
- protected NamedList<Object> requestWithBaseUrl(
- String baseUrl, SolrRequest<?> solrRequest, String collection)
+ @Override
+ public NamedList<Object> requestWithBaseUrl(
Review Comment:
change of argument order. Was protected, now public implementing the
abstraction.
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java:
##########
@@ -613,20 +609,11 @@ public NamedList<Object> request(SolrRequest<?>
solrRequest, String collection)
}
}
- /**
- * Executes a SolrRequest using the provided URL to temporarily override any
"base URL" currently
- * used by this client
- *
- * @param baseUrl a URL to a root Solr path (i.e. "/solr") that should be
used for this request
- * @param collection an optional collection or core name used to override
the client's "default
- * collection". May be 'null' for any requests that don't require a
collection or wish to rely
- * on the client's default
- * @param req the SolrRequest to send
- */
- public final <R extends SolrResponse> R requestWithBaseUrl(
- String baseUrl, String collection, SolrRequest<R> req)
+ @Override
+ public NamedList<Object> requestWithBaseUrl(
Review Comment:
This is a backwards incompatible signature change. This method was added
midway through 9.x and here I definitely want to change it to return a
NamedList. Defined at HttpSolrClientBase level
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java:
##########
Review Comment:
fyi @jdyer1
There's still a TODO that is maybe obsolete or something above
##########
solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java:
##########
@@ -291,6 +292,15 @@ public final T process(SolrClient client, String
collection)
return typedResponse;
}
+ /**
+ * @lucene.experimental
+ */
+ public final T processWithBaseUrl(HttpSolrClientBase client, String url,
String collection)
Review Comment:
Added this in kind with other process method that returns `T`. This enables
most users of the signature change I did to switch to this instead. I'm pretty
happy with this.
--
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]
