[
https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15170695#comment-15170695
]
Jason Gerlowski commented on SOLR-8097:
---------------------------------------
Thanks for the review Anshum. Sorry it took me a few days to get back to this.
Wanted to respond to your comments:
- re: {{updateLeadersOnly}}. Done; I've changed the name back to
{{updatesToLeaders}}.
- re: import cleanup. Done.
- re: Javadoc return tag. Didn't quite understand your comment. Looking
through the patch, I couldn't find the @return tag anywhere. Can you clarify
what you meant please?
- re: CUSC change being unrelated. I think this change is necessary. The CUSC
ctor that the change occurs in is now called from CUSCB (CUSCBuilder), which
may or may not have a real ExecutorService to pass in. Since CUSC needs an
ExecutorService, I changed to ctor to create its own if the param is null.
Hence those lines in the patch. I'm sure there's other ways to solve this
problem if there's something you don't like about those lines, but they are
related/required for the patch as it is now.
-re: BRP is now used as default in HSC/LBHSC. Looking at these classes, the
ctors don't take in a ResponseParser create their own BinaryResponseParser. So
BRP is already the default here. I had to make this explicit in the ctors that
take a ResponseParser, due to the same dynamic mentioned in the bullet point
above (That is, the ctor is now called from a builder, where responseParser may
or may not have actually been set). So I think this change is also
related/required for this patch.
> 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
>
>
> 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: [email protected]
For additional commands, e-mail: [email protected]
