dsmiley commented on code in PR #4799:
URL: https://github.com/apache/solr/pull/4799#discussion_r3844065024


##########
solr/solrj-streaming/src/java/org/apache/solr/client/solrj/io/stream/SolrStream.java:
##########
@@ -68,18 +69,52 @@ public class SolrStream extends TupleStream {
   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 Base URL of the stream.
-   * @param params Map<String, String> of parameters
+   * @param baseUrl 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;
     this.params = params;
   }
 
-  SolrStream(String baseUrl, SolrParams params, String core) {
-    this(baseUrl, params);
+  // 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
+   *     "http://host:8983/solr/myCore";.
+   * @param path the request handler path to query (e.g. "/export"). If not 
provided, defaults to
+   *     "/select".
+   * @param params query-parameters sent with the streaming request
+   */
+  public SolrStream(String baseUrl, String path, SolrParams params) {

Review Comment:
   Just looking at the types, it's fundamentally ambiguous.  The call-sites in 
Java don't dispatch by name, it dispatches by type so as a reader/reviewer we 
only hope that the arguments match the intent of the method's docs & var names. 
 
   
   It would placate me to mark this method `@VisibleForTesting`.  If that works 
for you, this is what I most prefer.
   
   My proposal collectionOrPath would provide a way for it to work both ways -- 
both would be valid.



-- 
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]

Reply via email to