gerlowskija commented on code in PR #3238:
URL: https://github.com/apache/solr/pull/3238#discussion_r2001404933
##########
solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java:
##########
@@ -185,8 +193,21 @@ public void setQueryParams(Set<String> queryParams) {
this.queryParams = queryParams;
}
- /** This method defines the type of this Solr request. */
- public abstract String getRequestType();
+ /**
+ * The type of this Solr request.
+ *
+ * <p>Pattern matches {@link SolrRequest#getPath} to identify ADMIN requests
and other special
+ * cases. Overriding this method may affect request routing within various
clients (i.e. {@link
+ * CloudSolrClient}).
+ */
+ public SolrRequestType getRequestType() {
+ String path = getPath();
+ if (path != null && CommonParams.ADMIN_PATHS.contains(path)) {
Review Comment:
> [re: 'qt' param] I almost removed it completely as one of my first big
actions as a committer
It's been on my list for awhile, as it's caused problems with the v2 API as
well. We really should nuke it across the board...
> The reason this is necessary at the moment is because of this pattern,
which is present in several unit tests
I think this pattern is mostly a result of gaps in SolrJ. We didn't offer a
"reload core" SolrRequest implementation at some point, so a test-author hacked
around it by forcing the request into a QueryRequest. But I wonder how many of
those gaps have closed up over time.
We have a reload-core SolrRequest now, on the v2 side at least:
`org.apache.solr.client.solrj.request.CoresApi.ReloadCore`. That should be
safe to use in tests. And even where there's still a SolrJ gap we should still
be able to replace could these QueryRequest mis-uses with GenericSolrRequest I
think?
Is there's not that many instances of the pattern, maybe that's a path
forward as well? (Though if it's too much scope creep, I'd also understand
that...)
--
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]
