Introduce GenericCollectionRequest #3423
Conversation
and deprecate GenericSolrRequest.setRequiresCollection
| new GenericSolrRequest( | ||
| SolrRequest.METHOD.GET, | ||
| "/schema/uniquekey", | ||
| SolrParams.of("collection", coll)) |
There was a problem hiding this comment.
I don't love that collection param; moved to the request path
| SolrRequest<SimpleSolrResponse> ur = | ||
| new GenericSolrRequest(METHOD.GET, "/get", params).setRequiresCollection(true); | ||
| var ur = | ||
| new GenericCollectionRequest(METHOD.GET, "/get", SolrRequest.SolrRequestType.ADMIN, params); |
There was a problem hiding this comment.
making the request type mandatory; force you to think about it. RTG is used for internal purposes; maybe QUERY would get throttled? Or maybe not; I recall the rate limiter would ignore internal requests. LBSolrClient should retry a QUERY but not ADMIN; maybe this should be QUERY.
There was a problem hiding this comment.
shouldn't be in this PR; I'll remove
| .setRequiresCollection(true)); | ||
| new GenericCollectionRequest( | ||
| SolrRequest.METHOD.GET, | ||
| "/tasks/list", |
There was a problem hiding this comment.
it was a little eye-opening to see we have a handler that doesn't "look" like an admin handler but is one
| client.request(req, testCollection); | ||
| + "{\"name\":\"film_vector\",\"type\":\"knn_vector_10\",\"indexed\":true,\"stored\":true}]}") | ||
| .getBytes(StandardCharsets.UTF_8), | ||
| XMLResponseWriter.CONTENT_TYPE_XML_UTF8), |
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
perhaps controversial as you added this @gerlowskija but the opinionated stance I'm taking is to build this into the type, not be an optional method to call or forget to call
There was a problem hiding this comment.
Aren't we attempting over time to get away fro the use of GenericSolrRequest anyway?
There was a problem hiding this comment.
Maybe but that's a long way out.
| import org.apache.solr.common.util.NamedList; | ||
|
|
||
| /** | ||
| * A generic request for sending to a Solr collection or core. |
There was a problem hiding this comment.
Seems a little sad that we still have the "collection or core" concept. I liked reading over the code, and the explicitness of the collection name being required. I also liked that the explicit type of request with the ADMIN was being added.
|
Not sure if this PR should refrain from changing RequestType or POST/GET for that matter. |
and deprecate GenericSolrRequest.setRequiresCollection
https://issues.apache.org/jira/browse/SOLR-XXXXX TODO
Trying to emphasize the collection-ness in the constructor, and make other aspects mandatory (request type).
In a separate PR but debatably same JIRA, I'll change many usages of QueryRequest that should not be using such; should be generic. I have that shelved, waitng for this. That's how I got started down this path, actually.