vyatkinv commented on PR #4320:
URL: https://github.com/apache/solr/pull/4320#issuecomment-4364211976
I’ve addressed most of the review comments. Summary of changes in the latest
commits:
1. Renamed `solrCloud` parameter to `solrConnection`.
2. Switched Streams to use `CloudSolrClientConnection` instead of a raw
string.
To do this, I broke backward compatibility for constructors that accepted
parameters directly. I believe this is acceptable since end users are not
expected to instantiate stream implementations directly—they typically pass
string expressions into `StreamFactory`. Most of these constructors are used
internally, in tests, or not used at all.
If needed, I can restore the old constructors as `@Deprecated` and
delegate them to the new ones based on `CloudSolrClientConnection`.
3. Fixed minor issues related to formatting and parameter ordering.
4. Renamed `getSolrConnection` to `buildSolrConnection`.
5. Added a new method to `CloudSolrClient.Builder` so internal Solr code can
use the typed builder, while keeping the string-based API for end users.
For the same reason, I replaced the string-based `getCloudSolrClient`
with a typed variant.
6. One concern: currently `zkHost` can accept HTTP URLs.
It seems more consistent if:
* `zkHost` only allows ZooKeeper connection strings,
* Solr URLs only allow HTTP(S),
* and the new `solrConnection` supports both.
Should we add validation in `StreamTool`, the JDBC driver, and
`buildSolrConnection()` to enforce that `zkHost` is strictly ZooKeeper? And
allow both formats only via the new parameter?
7. Renamed parameter-building method to `buildSolrParamsExcept` and
refactored it to remove the `Map<String, String>` variant, reusing the unified
implementation.
As follow-up work (either separate issue or PR), I suggest:
a) adding support for HTTP quorum in the StreamTool CLI
b) adding support for HTTP quorum in the JDBC driver
PS: Documentation and new tests are not added yet.
--
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]
