dsmiley commented on code in PR #4799:
URL: https://github.com/apache/solr/pull/4799#discussion_r3844079841
##########
solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/SolrStream.java:
##########
@@ -64,34 +65,33 @@ public class SolrStream extends TupleStream {
private boolean distrib = true;
private String user;
private String password;
- private String core;
private transient SolrClientCache clientCache;
private transient boolean doCloseCache;
// TODO SOLR-17995 proposes that we should deprecate this constructor in
favor of one of the other
// constructors that requires users to provide the core as an explicit
parameter
/**
- * @param baseUrl URL of the Solr core or collection to query, typically of
the form
+ * @param collectionOrCoreUrl URL of the Solr core or collection to query,
typically of the form
* "http://host:8983/solr/myCore";.
* @param params query-parameters sent with the streaming request
*/
- public SolrStream(String baseUrl, SolrParams params) {
- this.baseUrl = baseUrl;
+ public SolrStream(String collectionOrCoreUrl, SolrParams params) {
+ this.baseUrl = collectionOrCoreUrl;
Review Comment:
can we further check that there are exactly 2 '/' further validation?
##########
solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/SolrStream.java:
##########
@@ -106,15 +106,16 @@ public SolrStream(String baseUrl, SolrParams params,
String core) {
/**
* @param baseUrl the Solr node's "base" URL (i.e. no core or collection in
the path
* @param core the name of the collection or core to query; must be hosted
at {@code baseUrl}
- * @param path the request handler path to query (e.g. "/export"). If not
provided, defaults to
- * "/select".
+ * @param path the request handler path to query (e.g. "/export"). If not
provided (i.e. {@code
+ * null}), the handler is instead resolved from a "qt" param embedded in
{@code params}, or
+ * defaults to "/select" if no such param is present.
* @param params query-parameters sent with the streaming request
*/
public SolrStream(String baseUrl, String core, String path, SolrParams
params) {
this.baseUrl = baseUrl;
this.core = core;
Review Comment:
IMO ideally we would do a null check on core and if null then split it out
of baseUrl
--
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]
