lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jason Gerlowski (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
Date Sat, 27 Feb 2016 18:41:18 GMT

    [ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15170695#comment-15170695
] 

Jason Gerlowski edited comment on SOLR-8097 at 2/27/16 6:40 PM:
----------------------------------------------------------------

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.




was (Author: gerlowskija):
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message