epugh commented on PR #4114: URL: https://github.com/apache/solr/pull/4114#issuecomment-3886351131
> TL;DR: Yes. We probably need some additional change in order to have `CoordinatorHttpSolrCall` to work with this change > > Details: By debugging, it's shown this change right now would cause some issues in `HttpShardHandler#prepDistributed` as suspected. > > This is because `CoordinatorHttpSolrCall` only wraps the request by providing an overriding `getCloudDescriptor` with info of the actual core (with collection name, replica type and core shard ID replaced with the actual core info, instead of the synthetic core). Which means with this current change, getting `CloudDescriptor` from the `CoreDescroptor` would have returned the synthetic core with incorrect collection name (of the Synthetic core/collection) > > Perhaps in `CoordinatorHttpSolrCall`, we will need to instead override `getCore` to return a wrapped `SolrCore` and `CoreDescriptor` that its `getCloudDescriptor` would return the modified cloud description from the coordinator code? > > Not sure if I missed out any obvious details here 😓 Can we make sure to add a test that fails with this change? I worry as I do refactoring, that without David's sharp eyes, something like this would be merged based on the tests passing ;-). @patsonluk I hope you have some time to get the changes made, feel free to push to this branch. I'm really pushing hard to remove deprecated code from Solr, and this is one of them! -- 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]
