gerlowskija commented on code in PR #3423:
URL: https://github.com/apache/solr/pull/3423#discussion_r2236712808
##########
solr/core/src/java/org/apache/solr/cli/ExportTool.java:
##########
@@ -253,11 +253,12 @@ void fetchUniqueKey() throws SolrServerException,
IOException {
.build();
NamedList<Object> response =
solrClient.request(
- new GenericSolrRequest(
- SolrRequest.METHOD.GET,
- "/schema/uniquekey",
- SolrParams.of("collection", coll))
Review Comment:
+1, I think we've talked elsewhere about moving away from these `collection`
params across the board for APIs that also put the coll/core in the path.
##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/GenericSolrRequest.java:
##########
@@ -85,7 +85,9 @@ public GenericSolrRequest(METHOD m, String path,
SolrRequestType requestType, So
* 'false' otherwise.
*
* @param requiresCollection true if a default collection should be used,
false otherwise.
+ * @deprecated use {@link GenericCollectionRequest} instead
*/
+ @Deprecated
Review Comment:
I don't mind this method going away, per se.
My only concern here is that `GenericCollectionRequest` (as-written) assumes
v1. So I'm not sure how a user would make a collection-requiring v2 API call
without the use of this method.
This ties into the larger concern I posted at the PR-level: these axes
(v1/v2, collection-requiring or not) cause a combinatorial problem that gets
really unwieldy if we try to handle it without relying on setters or ctor
params or something similar to flatten some of the dimensions.
##########
solr/core/src/test/org/apache/solr/search/TestTaskManagement.java:
##########
@@ -185,8 +186,11 @@ private NamedList<String> listTasks() throws
SolrServerException, IOException {
cluster
.getSolrClient(COLLECTION_NAME)
.request(
- new GenericSolrRequest(SolrRequest.METHOD.GET, "/tasks/list")
- .setRequiresCollection(true));
+ new GenericCollectionRequest(
+ SolrRequest.METHOD.GET,
+ "/tasks/list",
Review Comment:
What in your mind makes something "look" like an admin handler?
That it should have "admin" in the path? Something else? (Not objecting or
disagreeing, just curious for more specifics here)
##########
solr/solrj/src/java/org/apache/solr/client/solrj/request/GenericCollectionRequest.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.client.solrj.request;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.Objects;
+import org.apache.solr.client.solrj.response.SimpleSolrResponse;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+
+/**
+ * A generic request for sending to a Solr collection or core.
+ *
+ * @see GenericSolrRequest
+ */
+public class GenericCollectionRequest extends
CollectionRequiringSolrRequest<SimpleSolrResponse> {
Review Comment:
[0] Conceptually, this seems like a subset or a special case of the larger
"Generic Solr Request" umbrella, so it feels a little unintuitive to me that
there's no inheritance or formal relationship between this class and
`GenericSolrRequest` and/or `GenericV2SolrRequest`.
Did you consider using one of those as a parent-class, before discarding
that approach? Or was `CollectionRequiringSolrRequest` your first instinct
inheritance-wise?
--
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
