[
https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15188773#comment-15188773
]
Anshum Gupta commented on SOLR-8097:
------------------------------------
Thanks Jason. Looks good overall.
* Change maybeCreateHttpSolrClientWithBuilder to getNewSolrClient() ? 'maybe'
makes it sound like a client may/may not be created. Also, with a method called
getNewSolrClient(), we could just change it to always construct a new client
object using the new design, without having to rename the method to a
non-maybe* pattern. Also, we passing the 'random()' isn't really required.
* Deprecation to the non-builder calls. Ideally, we should move the builder
classes to be static inner classes for the existing Client implementations.
Then we could switch everything to private and leave out the Builder exposed
when we want to remove the builder, rather than moving the code around.
* CUSC, and HttpSolrClient changes are unrelated
* Do you plan on adding more tests to ConcurrentUpdateSolrClientBuilderTest ?
* There are a few unused imports, we can clean them out before committing.
bq. I'm happy to move this in either direction (remove the
random-client-creation changes vs. expanding the changes to encompass other
SolrClient types)
Let's have a getter for randomizing client creation, while keeping the concept
of randomizing transparent i.e. the calling code doesn't ever know when we
randomize. Also, let's have other clients covered too.
I'll create a sub-task so we don't forget that we intend to rename
updatesToLeaders to preferLeaders. Feel free to create sub-tasks for everything
that you think is related but doesn't need to go with this.
> Implement a builder pattern for constructing a Solrj client
> -----------------------------------------------------------
>
> Key: SOLR-8097
> URL: https://issues.apache.org/jira/browse/SOLR-8097
> Project: Solr
> Issue Type: Improvement
> Components: SolrJ
> Affects Versions: master
> Reporter: Hrishikesh Gadre
> Assignee: Anshum Gupta
> Priority: Minor
> Attachments: SOLR-8097.patch, SOLR-8097.patch, SOLR-8097.patch,
> SOLR-8097.patch, SOLR-8097.patch, SOLR-8097.patch, SOLR-8097.patch,
> SOLR-8097.patch
>
>
> Currently Solrj clients (e.g. CloudSolrClient) supports multiple constructors
> as follows,
> public CloudSolrClient(String zkHost)
> public CloudSolrClient(String zkHost, HttpClient httpClient)
> public CloudSolrClient(Collection<String> zkHosts, String chroot)
> public CloudSolrClient(Collection<String> zkHosts, String chroot, HttpClient
> httpClient)
> public CloudSolrClient(String zkHost, boolean updatesToLeaders)
> public CloudSolrClient(String zkHost, boolean updatesToLeaders, HttpClient
> httpClient)
> It is kind of problematic while introducing an additional parameters (since
> we need to introduce additional constructors). Instead it will be helpful to
> provide SolrClient Builder which can provide either default values or support
> overriding specific parameter.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
