epugh commented on code in PR #5202:
URL: https://github.com/apache/solr/pull/5202#discussion_r4358763881


##########
solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-11.adoc:
##########
@@ -41,3 +41,10 @@ bin/solr start -Dsolr.node.roles=data:on,overseer:preferred
 
 A node started this way asks the Overseer to re-run its node prioritization, 
so a preferred node takes over without waiting for the current Overseer to 
restart.
 Note that node roles are fixed for the lifetime of a node: unlike `ADDROLE`, 
they cannot be changed on a running node.
+
+=== `collection` Parameter No Longer Routes Requests

Review Comment:
   so I guess I am slightly unclear, it feels to me like the `collection` 
parameter is no longer needed?



##########
solr/solr-ref-guide/modules/deployment-guide/pages/solrcloud-distributed-requests.adoc:
##########
@@ -276,12 +276,15 @@ Note that if you provide these parameters in a query 
string, they need to be pro
 
 === collection Parameter
 
-The `collection` parameter allows you to specify a collection or a number of 
collections on which the query should be executed.
-This allows you to query multiple collections at once and the features of Solr 
which work in a distributed manner will work across collections.
+The collection named in the URL path governs which collections a query runs 
against.
+To query several collections at once, name them in the path, as a 
comma-separated list or through an alias; the features of Solr which work in a 
distributed manner will work across those collections.
+
+The `collection` parameter can only narrow the set named in the path.

Review Comment:
   if we are dropping the use of the collection parameter?  Which I think it 
what we ant to be doing right?   Then we don't need to document it or show it.  
   
   Or, are we deprecating the "collection" parameter and changing it's behavior?



##########
solr/core/src/java/org/apache/solr/api/V2HttpCall.java:
##########
@@ -132,9 +132,8 @@ public void call(SolrQueryRequest req, SolrQueryResponse 
rsp) {
       if (pathSegments.size() > 1 && ("c".equals(prefix) || 
"collections".equals(prefix))) {
         origCorename = pathSegments.get(1);
 
-        String collectionStr = queryParams.get(COLLECTION_PROP, origCorename);
-        collectionsList =
-            resolveCollectionListOrAlias(collectionStr); // &collection= takes 
precedence
+        String collectionStr = origCorename;

Review Comment:
   I suppose teh `collectionStr` sort of better conveys the werid remapping of 
what looks like a core to a collection.      
   
   I wonder, is there a better name than `origCorename` that would somehow 
convey that we cover both core and collection?   Maybe I have seen 
`coreOrCollection` ?



##########
solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java:
##########
@@ -1538,16 +1538,20 @@ private void testMultipleCollections() throws Exception 
{
             .getNumFound();
 
     SolrQuery query = new SolrQuery("*:*");
-    query.set("collection", "collection2,collection3");
-    long found = clients.get(0).query(query).getResults().getNumFound();
+    long found = clients.get(0).query("collection2,collection3", 
query).getResults().getNumFound();

Review Comment:
   interesting, so I can just put in the path multiple collections comma 
delimited...   how not restful is that?  or is that okay?  If it was a concept 
we supported, well, there are bunch of APIs that might benefit by "comma 
dellimited list in a path param"...    s



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