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

Reply via email to