[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Anshum Gupta updated SOLR-8097: --- Fix Version/s: (was: 6.2) 6.1 > 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: 6.0 >Reporter: Hrishikesh Gadre >Assignee: Anshum Gupta > Fix For: 6.1, master (7.0) > > 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, 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 zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Steve Rowe updated SOLR-8097: - Fix Version/s: (was: 6.0) > 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: 6.0 >Reporter: Hrishikesh Gadre >Assignee: Anshum Gupta > Fix For: 6.1 > > 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, 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 zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Anshum Gupta updated SOLR-8097: --- Priority: Major (was: Minor) > 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 > Fix For: master, 6.1 > > 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, 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 zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Anshum Gupta updated SOLR-8097: --- Fix Version/s: 6.1 master > 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 > Fix For: master, 6.1 > > 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, 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 zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Anshum Gupta updated SOLR-8097: --- Attachment: SOLR-8097.patch Here's an updated patch with renaming. I plan on getting this into master later today. > 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, 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 zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jason Gerlowski updated SOLR-8097: -- Attachment: SOLR-8097.patch Uploading a patch that has everything except the renames. (See my comment above for more information about the Builder renames) > 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, 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 zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[
https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jason Gerlowski updated SOLR-8097:
--
Attachment: SOLR-8097.patch
Ok, this removes all* uses of the now-deprecated constructors.
The asterisk above implies there are still a few cases where the constructors
are called.
- In {{SolrTestCaseJ4}}, where there are utility methods that randomly choose
whether to create clients using the builder, or the now-deprecated ctor.
- There are a few classes which have a private SolrClient subclass. In almost
all cases this is done to specify some alternate behavior for
{{SolrClient.handleError()}}. I changed these all to use the "largest" ctor
for the SolrClient they're extending (the ctor with all parameters, that will
be kept around with a reduced visibility after the other ctors are deleted.
I'm not sure I described the second case well, but if my description is
unclear, check out {{StreamingSolrClients.java}}, which has a good example of
this.
All other uses have been removed.
As a side note, in this last revision, I realized that deprecating, eventually
removing, and lowering the visibility of SolrClient ctors makes implementations
difficult to extend anonymously. This isn't a big deal. Just wanted to
mention it in case it bothers anyone.
This should be ready to go [~anshumg]!
> 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, 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 zkHosts, String chroot)
> public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jason Gerlowski updated SOLR-8097: -- Attachment: SOLR-8097.patch Thanks for the review Anshum. Made all the changes you suggested with 1 or 2 exceptions: - I wanted to add more tests to ConcurrentUpdateSolrClientBuilderTest, but for some reason ConcurrentUpdateSolrClient lacks many of the getters that other SolrClients have. I wrote tests for everything that has getters, but for ConcurrentUpdateSolrClient that's not that much. - The change to CUSC wasn't unrelated. The CUSC builder now calls that ctor in such a way that the ExecutorService _could_ be null. So the CUSC builder now needs to be able to handle that case. (We actually already discussed this, just leaving this note for completeness.) Other than that, I've made all the changes you suggested. All tests pass locally. Should be ready to go (or at least, review again). Thanks! > 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, 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 zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jason Gerlowski updated SOLR-8097: -- Attachment: SOLR-8097.patch Updates patch on top of recent changes in master. Tests pass locally. Should be ready for review. The only/main open question on this that I know of is "testing". Right now the patch changes many tests to randomly choose between (1) normal creation and (2) using the new builder when creating HttpSolrClient instances. It does nothing for other SolrClient implementations (excepting unit tests on the builders themselves, which are included). It was kindof unclear to me whether these changes were desired or not. I'm happy to move this in either direction (remove the random-client-creation changes vs. expanding the changes to encompass other SolrClient types), based on how people would like this to be structured. Looking for some feedback here in particular; I don't know how to move forward until the testing approach either gets a "thumbs up" or "thumbs down". > 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 zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jason Gerlowski updated SOLR-8097: -- Attachment: SOLR-8097.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, 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 zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[
https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jason Gerlowski updated SOLR-8097:
--
Attachment: SOLR-8097.patch
Oops, the carelessness of {{git add -A}} (and the user who ran it) strikes
again.
Sorry for wasting your time. The updated patch should work.
> 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
>
>
> Currently Solrj clients (e.g. CloudSolrClient) supports multiple constructors
> as follows,
> public CloudSolrClient(String zkHost)
> public CloudSolrClient(String zkHost, HttpClient httpClient)
> public CloudSolrClient(Collection zkHosts, String chroot)
> public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[
https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jason Gerlowski updated SOLR-8097:
--
Attachment: SOLR-8097.patch
bq. Let's carry on with adding tests, javadocs, deprecation, and also using
this in existing tests. We might not want to change all the tests but
optionally pick between builder and existing way of constructing the clients
for now in the tests though.
Time for a short check-in on making the revisions you suggested [~anshumg]:
- tests? check.
- javadocs? check.
- deprecation? nope.
- using in existing tests? In progress (see below).
The attached patch changes all tests that previously created {{HttpSolrClient}}
objects directly. With this patch, these tests now either use the builder, or
create the objects directly, based on the tests' {{random()}} value. Is that
about what you were thinking of when you suggested this Anshum?
Should this (or a similar change) be made to all test files that create
SolrClients, or just enough of the tests to get decent exposure for the
builder? I'd push for creating SolrClients uniformly in all the tests, though
that does make this patch/commit much more burdensome in many aspects (keeping
it up to date, reviewing it, etc.). So I see arguments either way. I'll
revise the changes in the HttpSolrClient-consuming tests, and make similar
changes for tests consuming the other types of SolrClients once I hear any
thoughts that others might have.
> 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
>
>
> Currently Solrj clients (e.g. CloudSolrClient) supports multiple constructors
> as follows,
> public CloudSolrClient(String zkHost)
> public CloudSolrClient(String zkHost, HttpClient httpClient)
> public CloudSolrClient(Collection zkHosts, String chroot)
> public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jason Gerlowski updated SOLR-8097: -- Attachment: SOLR-8097.patch Adds tests for each builder. Removes abstract-class approach. Still need to push usage of builders into other SolrJ tests. Coming soon hopefully. > 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 > > > Currently Solrj clients (e.g. CloudSolrClient) supports multiple constructors > as follows, > public CloudSolrClient(String zkHost) > public CloudSolrClient(String zkHost, HttpClient httpClient) > public CloudSolrClient(Collection zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[ https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jason Gerlowski updated SOLR-8097: -- Attachment: SOLR-8097.patch Puts patch on top of latest trunk/master. Fixes some of Anshum's concerns. Still needs tests, fixes based on comments above before it's ready for another review. But I wanted to push up the changes I have so far at least. > 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 > > > Currently Solrj clients (e.g. CloudSolrClient) supports multiple constructors > as follows, > public CloudSolrClient(String zkHost) > public CloudSolrClient(String zkHost, HttpClient httpClient) > public CloudSolrClient(Collection zkHosts, String chroot) > public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[
https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jason Gerlowski updated SOLR-8097:
--
Attachment: SOLR-8097.patch
Ok, so after a bit of a delay, I've got an updated patch which attempts to
incorporate Shawn and Anshum's input.
h3. In This Patch
- builder types for CloudSolrClient, LBHttpSolrClient, HttpSolrClient, and
ConcurrentUpdateSolrClient.
- two abstract types which enabled some sharing of setters:
{{HttpClientBasedSolrClientBuilder}}, and
{{ResponseParserBasedSolrClientBuilder}}. I say _some_ , because while the
inheritance-chain-to-allow-shared-setters sounded like a great idea, it turned
out to be tough in practice. Only one param was present in all builders
(HttpClient). Others were more hit-and-miss, which made getting good re-use
hard in Java's single-inheritance world.
h3. Not In This Patch
- Good Javadocs
- Much testing.
- Any deprecation notes.
- Internal usage of these builder types in SolrJ.
I plan on eventually adding items in the list above, but I wanted to get some
feedback on the builder class structure before continuing. [~elyograg], are
you happy with the re-use I was able to get from things here? It was your
suggestion, so just wanted to make sure I was faithful to your idea and didn't
misinterpret what you wanted.
(Patch is on top of latest trunk.)
> 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
>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 zkHosts, String chroot)
> public CloudSolrClient(Collection 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
[jira] [Updated] (SOLR-8097) Implement a builder pattern for constructing a Solrj client
[
https://issues.apache.org/jira/browse/SOLR-8097?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jason Gerlowski updated SOLR-8097:
--
Attachment: SOLR-8097.patch
I've attached a first pass at the creation of a {{CloudSolrClientBuilder}}
class. The attached patch contains the builder class, an added
{{CloudSolrClient}} ctor that takes all parameters, and some basic tests for
the builder and new ctor.
Some notes on the patch:
- patch is off of trunk/master. I assume we'd also want it in 5.x, but one
thing at a time...
- created two small helper methods in {{CloudSolrClient}}. Maybe I
shouldn't've done that, but I found myself copy-pasting the code, so it seemed
like the right thing to do at the time. Happy to change if necessary.
- I didn't change any other code in the project to actually _use_ the new
builder. I'm happy to do this, just wasn't sure it was the right thing and
wanted to wait on feedback from others.
- I didn't touch (delete, re-scope, deprecate, etc.) any of the existing ctors.
I assume an eventual version of this patch on 5.x should deprecate the
existing ctors. Should I also have the ctors be deprecated in the trunk/master
version? Or should I do something more aggressive, like deleting them
altogether? Happy to do whatever; just wasn't entirely sure what the right
move was (even after [~elyograg] addressed my earlier, less directed questions
above...thanks btw).
All-in-all, the patch still needs a bit of work, but should be a solid start.
Looking forward to hearing what people think/want-changed!
> 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: Trunk
>Reporter: Hrishikesh Gadre
>Priority: Minor
> Attachments: 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 zkHosts, String chroot)
> public CloudSolrClient(Collection 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
