chan-dx commented on code in PR #4829: URL: https://github.com/apache/solr/pull/4829#discussion_r3944950124
########## solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java: ########## Review Comment: Agreed, both now on [HttpSolrClient - lines 370](https://github.com/apache/solr/pull/4829/changes#diff-ca5ca7cfb5e21eb3d799cd81f39cfd8b357654b713e195eb91d532368f6c122bR370-R383), overridden per transport. `CloudSolrClient`'s overrides are gone. It calls `getHttpClient().wasCommError(...) `directly [CloudSolrClient - lines 712](https://github.com/apache/solr/pull/4829/changes#diff-2be5ee044b360932677ad117291881853913b1fbe72bc48abc05b968530fb2aaR712). **Notes:** - The `false` default on `SolrClient` was what let the LB ask without an `instanceof`, per the sketch on SOLR-18402; with it gone I narrowed `getClient(Endpoint)` to return `HttpSolrClient`. No in-tree change: `Builder<C extends HttpSolrClient>` and `LBAsyncSolrClient.getClient` already guaranteed it. However, an out-of-tree subclass declaring `SolrClient` gets `AbstractMethodError` until it recompiles. Reachable, since `LBSolrClient(List<Endpoint>) `bypasses the Builder; my own test fixture had to change. Happy to reverse it if you'd rather. - If the new `getClient` signature stands, let me know if you want a line in `major-changes-in-solr-10.adoc` in case anyone subclasses it out-of-tree? - That also made the private `doRequest` helper's `instanceof` constant-true and its fallback unreachable, and its `// TODO SOLR-17541` was already stale. Deleted in its own commit. Happy to drop that commit if you'd rather keep this PR narrower. -- 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]
