SOLR-17243: CloudSolrClient support for req.getBasePath

[dsmiley]

Now CloudSolrClient can be used to route requests that specify a specific URL. LBSolrClient has never supported this and shouldn't.

https://issues.apache.org/jira/browse/SOLR-17243

[dsmiley]

FYI @iamsanjay this will be useful for the transition to Jetty based clients which also involves less one-off client creations. I have WIP that will use this.

[gerlowskija]

Left a few minor comments if you do choose to go forward with this - a suggested javadoc changes, caught a typo, etc. All very minor.

My main concern here is conceptual: CloudSolrClient is the client that folks use if they want all the load-balancing and auto-routing and "intelligence" build in. If a user wanted to send a request to a particular end-point, why not create their own Http[2]SolrClient. Or better yet, just pull an existing client out of CSC through the getters that CSC makes available i.e. cloudClient.getLBClient().getClient(...)

(Note that this isn't a -1, just some concerns.)

[gerlowskija]

😬

Not an issue for this PR, but man, this is kindof scary to see written out.

[dsmiley]

I'm airing dirty laundry :-)

[gerlowskija]

[0] Maybe tweak the wording here to make it a little clearer what value this method expects for requests that don't target a particular core/collection.

Perhaps something like:

The HTTP path suffix used when sending this request.

Commonly used with {@link #getBasePath()} and {@link #getCollection()} to assemble the full HTTP path for the request.

For requests targeting a specific core or collection, callers need only specify those path components that follow the collection name (e.g. "/select" for the request "/solr/coreName/select"). This is sometimes called the "handler" path.

For requests that don't target a particular core or collection, callers should specify any path components that follow "/solr" or "/api" (e.g. "/admin/info/health" in "/solr/admin/info/health").

Feel free to ignore this advice though - any javadocs here are an improvement.

[dsmiley]

If a user wanted to send a request to a particular end-point, why not create their own Http[2]SolrClient. Or better yet, just pull an existing client out of CSC through the getters that CSC makes available i.e. cloudClient.getLBClient().getClient(...)

Oh right, I forgot to share I have the same concern. But I'd like a convenience method like CloudHttpSolrClient.getHttpSolrClient(String url). How does that sound?

[dsmiley]

I think as long as QueryRequest continues to have a getBasePath, there is probably a fundamental conceptual issue with some clients supporting it and some not; it doesn't feel well placed on this abstraction. Furthermore it sucks that LBSolrClient mutates this path during processing. I would prefer to see it removed, and instead only HttpSolrClient should support a special request method overloaded that accepts a URL to the node to where the request should be sent. LBSolrClient would use this method instead of mutating setBasePath, and furthermore all the other callers of setBasePath out there would instead use this pattern. I just scanned over all of them outside tests and nearly every one submits the request with a client immediately following calling setBasePath. Again, this suggests that Http2SolrClient should have such an overload.

[gerlowskija]

I forgot to share I have the same concern. But I'd like a convenience method like CloudHttpSolrClient.getHttpSolrClient(String url). How does that sound?

Ah, glad it's not just me. I think a convenience method is a solid approach here - if you're not familiar with the LB client, cloudClient.getLBClient().getClient(...) is pretty unintuitive, so a convenience method would make that possibility more visible to folks.

I could quibble about the method name - I'd opt for something that focuses on conveying the usecase instead of restating the method return type. e.g. getClientForUrl(...). But that's just my 2c, and I don't have strong opinions there.

[dsmiley]

As I said in my last comment, I'd prefer to abandon QueryRequest.getBasePath and setBasePath. https://issues.apache.org/jira/browse/SOLR-17256
It'd still be nice to expose an HttpSolrClient out of CloudSolrClient but my motivation for doing so may be handled by SOLR-17256 so this PR here may not be needed anyway. I'll take the javadocs though :-)