[jira] [Commented] (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:comment-tabpanel=15506475#comment-15506475 ] Jason Gerlowski commented on SOLR-8097: --- I created SOLR-9535 to address the recent concern brought up here, and attached a small patch containing the fix. > 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] [Commented] (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:comment-tabpanel=15489278#comment-15489278 ] Anshum Gupta commented on SOLR-8097: >From how I'm looking at this, it's a bug really as it took away a capability >that existed in previous releases and this was not a planned move. I think >it'd make sense for us to push this with 6.2.1 if we're sure that the only way >is to change the scope. > 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] [Commented] (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:comment-tabpanel=15489273#comment-15489273 ] Anshum Gupta commented on SOLR-8097: I can't recollect right now and I'm not looking at the code but I thought there was a way around it. I'll take your word for it here :). The main thing here was to make sure that the end user uses the builder. Converting the constructor to be protected doesn't defeat that purpose so I'm fine. > 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] [Commented] (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:comment-tabpanel=15487748#comment-15487748
]
Jason Gerlowski commented on SOLR-8097:
---
I'll create a separate issue for changing the appropriate SolrClient ctors to
be {{protected}}, and will push up a patch shortly.
I'll summarize the discussion here, and others can weigh in on alternate
approaches if they wish.
> 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] [Commented] (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:comment-tabpanel=15485839#comment-15485839 ] Shawn Heisey commented on SOLR-8097: bq. the Builder could be extended instead of extending the constructor itself I tried to extend the client in this way, adding a new setting to the derived client class and exposing it in the derived Builder, but ultimately it comes down to the same problem -- the "all parameters" constructor, which is the only one that will survive the transition to 7.0, cannot be used in an extended Client/Builder because it's private. One option, which I would not want to employ because it would involve duplicate code that will quickly become stale, is to copy all the code in the private constructor and paste it into the subclass, then add parameters as required and use that constructor in a derived Builder class. IMHO, the only sane option for experienced developers is to change the internal constructor from private to protected, allowing derivative classes to utilize it after doing class-specific setup. The developer will usually also need to extend the internal Builder class to expose configuration of any new capability. I like what we've done with the Builder, and I agree that after 7.0 removes deprecated code, the constructor should not be public ... but making it private is too limiting. > 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] [Commented] (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:comment-tabpanel=15485554#comment-15485554 ] Anshum Gupta commented on SOLR-8097: Right, we can open up the constructor for subclassing but I can't figure the need. I may be missing something here but the Builder could be extended instead of extending the constructor itself and I think that's the right way to go considering we'd be doing away with access to the constructors in 7.0 anyways. > 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] [Commented] (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:comment-tabpanel=15485336#comment-15485336 ] Shawn Heisey commented on SOLR-8097: What is your *specific* goal in creating a subclass? I ask because the X is usually more important than the Y. See this: http://people.apache.org/~hossman/#xyproblem A SolrClient object is a complex thing, particularly the Cloud version. Although we try to keep the public API from changing much in minor releases, the internal implementation is a VERY different story. Because the implementation can change dramatically from release to release, certain details are kept private. This reduces the risk of breaking user code. That said, there really is no reason we should *prevent* subclassing like we currently do, even if we recommend not doing it because it makes user code brittle. It makes sense to change the kitchen-sink constructor from private to protected. SOLR-8975 might be a good place to tackle this, but it might need its own issue. I think we should also recommend extending the Builder when subclassing. When 7.0 is released, all public constructors will be gone, and the Builder will be the *only* way to create a client object. > 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.2, 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] [Commented] (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:comment-tabpanel=15485083#comment-15485083 ] Perrin Bignoli commented on SOLR-8097: -- What was the result of that discussion? I am interested in creating a subclass of CloudSolrClient. I don't see how that is possible with the current code or if there are only private constructors. Other *SolrClient classes appear to have a protected "Builder" constructor. They also have external Builder classes (at least on Master). Is there a reason why CloudSolrClient is set up to prevent subclassing? Please let me know if I am missing something obvious. Also, that discussion does not involve member variable visibility, although that is probably outside of the scope of this particular ticket. > 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.2, 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] [Commented] (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:comment-tabpanel=15484908#comment-15484908 ] Anshum Gupta commented on SOLR-8097: Here's the discussion about that: https://issues.apache.org/jira/browse/SOLR-8097?focusedCommentId=15227338=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15227338 > 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.2, 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] [Commented] (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:comment-tabpanel=1548#comment-1548 ] Perrin Bignoli commented on SOLR-8097: -- Why is the the visibility of the following constructor in CloudSolrClient: private CloudSolrClient(Collection zkHosts, String chroot, HttpClient httpClient, LBHttpSolrClient lbSolrClient, boolean updatesToLeaders, boolean directUpdatesToLeadersOnly) set to private and not protected? > 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.2, 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] [Commented] (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:comment-tabpanel=15294070#comment-15294070 ] ASF subversion and git services commented on SOLR-8097: --- Commit 9645d4213292121d2f011f5440684ea25d7beaa3 in lucene-solr's branch refs/heads/branch_6_0 from Chris Hostetter [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=9645d42 ] SOLR-9028: Fixed some test related bugs preventing SSL + ClientAuth from ever being tested (cherry picked from commit 791d1e7) Conflicts: solr/core/src/test/org/apache/solr/cloud/SSLMigrationTest.java solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java solr/test-framework/src/java/org/apache/solr/util/SSLTestConfig.java Conflicts: solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java For branch_6_0: Since SOLR-8097 will land in 6.1, remove calls in TestMiniSolrCloudClusterSSL to SolrTestCaseJ4.getHttpSolrClient(). > 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.0, 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] [Commented] (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:comment-tabpanel=15280639#comment-15280639 ] Anshum Gupta commented on SOLR-8097: Yes but it was only fixed for 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.0, 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] [Commented] (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:comment-tabpanel=15280444#comment-15280444 ] Hoss Man commented on SOLR-8097: [~anshumg] - shouldn't this issue be resolved? > 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.0, 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] [Commented] (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:comment-tabpanel=15237870#comment-15237870 ] Jason Gerlowski commented on SOLR-8097: --- I created SOLR-8975 for this work, with a short description. I'll copy over some more of the details/suggestions/requirements you've described here shortly, and get started. Thanks for chiming in, let's continue this discussion over on SOLR-8975. > 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] [Commented] (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:comment-tabpanel=15237798#comment-15237798 ] Shawn Heisey commented on SOLR-8097: I personally would never change those things after the first use of a client -- not even the default collection on a cloud client. If the client is being used by multiple threads, determining what will happen if you change the client after you start using it is difficult. IMHO this makes all setters superfluous, even setDefaultCollection. Anything a setter does should be handled by the builder, and you can supply a collection parameter to methods that make requests, so you're never locked in to the default. +1 to open a new issue to deprecate the setters in branch_6x and remove in master, after making sure all options are handled by the builder. If the javadoc on the setters doesn't already say so, it should indicate that they are not thread-safe and should only be used immediately after object creation. > 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] [Commented] (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:comment-tabpanel=15237676#comment-15237676 ] Anshum Gupta commented on SOLR-8097: Collection, perhaps yes. But I don't change any of the others myself. At the highest level, for anything we do, we need to handle the following (at least): 1. If we are changing APIs, it's a deprecation, specially in minor versions and nothing is broken. 2. There are tests for the changed behavior We have handled those well here. Also, let's do that as a separate JIRA. I don't have strong opinions on that. > 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] [Commented] (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:comment-tabpanel=15237634#comment-15237634 ] Jason Gerlowski commented on SOLR-8097: --- I thought about doing that as a part of my patch, but it seemed like it might be handled best in a separate JIRA. I'd be happy to push up a patch with that change in a sub-task if it's one that people are interested in. One question I have is whether there's ever a valid use-case for changing/tweaking a SolrClient after creating it. (i.e. Does anyone ever change their RequestWriter, zkHost, collection, etc, instead of just creating a new client). I'm not implying that there is a good reason to do so, or that I'm against removing the setters (I'm all for it). (Just want to make sure, after the issue I partially caused in SOLR-8642) > 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] [Commented] (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:comment-tabpanel=15237456#comment-15237456 ] Anshum Gupta commented on SOLR-8097: Sure > 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] [Commented] (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:comment-tabpanel=15237124#comment-15237124 ] Shalin Shekhar Mangar commented on SOLR-8097: - Can we deprecate the various setters inside HttpSolrClient and remove them from master? Some of those are missing in the Builder too e.g. setRequestWriter > 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] [Commented] (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:comment-tabpanel=15232845#comment-15232845 ] ASF subversion and git services commented on SOLR-8097: --- Commit f479f16d3a8b57126560c19c57885a103360f1c3 in lucene-solr's branch refs/heads/branch_6x from [~anshum] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f479f16 ] SOLR-8097: Implement builder pattern design for constructing SolrJ clients and deprecate direct construction of clients > 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] [Commented] (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:comment-tabpanel=15231355#comment-15231355 ] Anshum Gupta commented on SOLR-8097: Thanks Jason and Shawn. I'll commit this to 6x tomorrow. > 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] [Commented] (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:comment-tabpanel=15231325#comment-15231325 ] ASF subversion and git services commented on SOLR-8097: --- Commit b02b026b7d979a9dac3c549c156dd9706e6fc5ba in lucene-solr's branch refs/heads/master from [~anshum] [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b02b026 ] SOLR-8097: Implement builder pattern design for constructing SolrJ clients and deprecate direct construction of clients > 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] [Commented] (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:comment-tabpanel=15228549#comment-15228549 ] Shawn Heisey commented on SOLR-8097: I didn't think of potential naming conflicts. Fully qualified references for all "Builder" classes other than the local one seems like a reasonable solution. Or we could pull the builders into separate source files, but I don't think that would really reduce verbosity by enough to matter. The only REAL advantage to separate classes is that I like the clean separation. The relationship between a class and its builder is pretty intimate, so embedding is understandable. I will look at your patch later, when I have more than a few minutes. Patches will be as big as necessary. Keeping the size down is a good idea when possible, but don't worry too much about the size. Just make sure that the patch only makes required changes; keep away from massive code reformats. > 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] [Commented] (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:comment-tabpanel=15228430#comment-15228430
]
Jason Gerlowski commented on SOLR-8097:
---
So, I've got an updated patch that does everything except the renaming. I'm
fine w/ shortening the names of the static *Builder classes, but when I went to
do that, I ran into a bunch of compilation issues related to "ambiguous
references" or "type mismatch".
The root of the problem appears to be name conflicts, most of which are with
HttpClient's {{Builder}} class. As an example, look at this snippet of code:
{code}
Builder requestConfigBuilder =
HttpClientUtil.createDefaultRequestConfigBuilder();
if (soTimeout != null) {
requestConfigBuilder.setSocketTimeout(soTimeout);
}
if (connectionTimeout != null) {
requestConfigBuilder.setConnectTimeout(connectionTimeout);
}
{code}
This will fail to compile with the message: "Type mismatch: Cannot convert from
RequestConfig.Builder to ConcurrentUpdateSolrClient.Builder". This is easily
fixable by changing all {{Builder}} references to be unambiguous
(RequestConfig.Builder or ConcurrentUpdateSolrClient.Builder). I'm happy to do
that, if this is what we want, but I'm reluctant to let this patch get much
bigger than it already is.
That said, I'm happy to pull the trigger if we're sure this is what we want.
If anyone else sees a better way around this that I'm missing, please let me
know.
> 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] [Commented] (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:comment-tabpanel=15227369#comment-15227369 ] Anshum Gupta commented on SOLR-8097: from my previous comment: bq. 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. I would have liked a separate class but for the purpose of back-compat and then having to move the code around when we let go of the compat, it makes sense to have the builder as an inner class. If you think *.Builder makes more sense than Builder, I could buy that. > 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] [Commented] (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:comment-tabpanel=15227338#comment-15227338 ] Shawn Heisey commented on SOLR-8097: Further research on this turns up both methods as valid -- including the builder class inside the class it's building, or as a separate class. The example that I found where it was an internal class just named it "Builder" -- which I think is wise if we are going to embed the class. In that case, we would not need *any* public or protected constructors, just a single private constructor that takes the Builder object. I do like the separate class idea, but I would not be opposed to HttpSolrClient.Builder instead. > 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] [Commented] (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:comment-tabpanel=15227291#comment-15227291 ] Shawn Heisey commented on SOLR-8097: We could completely eliminate all the public constructors in master. If a constructor is actually needed by the Builder classes, we would leave only one of them (the one with all the options) marked "protected". Because the Builder would be in the same package, it would have access to that constructor. I just noticed that the HttpSolrClientBuilder class does not have its own .java file. I think that the builders should be entirely separate classes. This is the pattern that HttpClient uses. I'm not sure whether this enough to veto the patch, but it's how I think we should do it. There's no way to know whether people are actually trying to extend specific SolrClient implementations, but I would not expect that to be common. A single protected constructor would allow some leeway in this regard. > 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] [Commented] (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:comment-tabpanel=15227044#comment-15227044 ] Anshum Gupta commented on SOLR-8097: Thanks Jason > 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] [Commented] (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:comment-tabpanel=15227040#comment-15227040 ] Jason Gerlowski commented on SOLR-8097: --- Yep, working on updating the patch now. It looks like someone's added a new HttpSolrClient ctor while this patch has been outstanding, so updating the patch is a little more involved now. Hope to have something up shortly still though. > 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] [Commented] (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:comment-tabpanel=15226950#comment-15226950 ] Anshum Gupta commented on SOLR-8097: I don't think that difficulty in anonymous extension of SolrClient should stop us from doing this. If no one else has a problem with this, I'll go ahead and commit. > 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] [Commented] (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:comment-tabpanel=15226947#comment-15226947 ] Anshum Gupta commented on SOLR-8097: [~gerlowskija] can you upda > 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] [Commented] (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:comment-tabpanel=15207366#comment-15207366 ] Anshum Gupta commented on SOLR-8097: We should not be using deprecated code for anything other than back-compat testing :) > 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] [Commented] (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:comment-tabpanel=15207360#comment-15207360 ] Jason Gerlowski commented on SOLR-8097: --- I didn't forget, but I was unsure whether that should be included in this JIRA or not. (I'm personally for changing the usage in production code, I was just being conservative.) I'll upload a patch shortly removing use of the deprecated ctors entirely. > 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] [Commented] (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:comment-tabpanel=15207314#comment-15207314 ] Anshum Gupta commented on SOLR-8097: Seems like you forgot to change the usage of deprecated constructors in the code. You changed it in the tests, but not the rest of the code base. The rest seems great. > 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] [Commented] (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:comment-tabpanel=15200383#comment-15200383 ] Anshum Gupta commented on SOLR-8097: I'll take a look at this 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 > > > 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] [Commented] (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:comment-tabpanel=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 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] [Commented] (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:comment-tabpanel=15179287#comment-15179287 ] Jason Gerlowski commented on SOLR-8097: --- Hmm, still seeing some test failures with this patch. Haven't narrowed down the problem yet, but I'm working on it. Not ready for review yet (other than general feedback on how to expose the new builders into existing tests). > 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] [Commented] (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:comment-tabpanel=15176410#comment-15176410 ] Jason Gerlowski commented on SOLR-8097: --- bq. we need to continue testing the current SolrJ implementation too Does RandomizedSolrClientCreator address your concerns there? I wrote it with that in mind, but I might've misinterpretted what you were asking for. It does a coin flip using each test's Random object, and does builder or non-builder SolrClient creation based on the result. I thought that was what you were suggested having a way to "optionally pick between builder and existing way" in your comment above. I'm happy to back out the changes to all the tests if you aren't happy with a coin-flip-creation approach here. In that case, should I only be changing a subset of the tests? > 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] [Commented] (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:comment-tabpanel=15176292#comment-15176292 ] Anshum Gupta commented on SOLR-8097: Seems like there's some issue with the patch as it has .orig files. Do you mind posting another one? > 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] [Commented] (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:comment-tabpanel=15176276#comment-15176276 ] Anshum Gupta commented on SOLR-8097: Thanks Jason. Looking at it now. Let's not move all the tests and have a massive patch. Also, we need to continue testing the current SolrJ implementation too i.e. non-builder pattern. > 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] [Commented] (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:comment-tabpanel=15170749#comment-15170749
]
Jason Gerlowski commented on SOLR-8097:
---
So, looking close at these builder classes, I'm not sure that either an
abstract-class, or an interface approach will allow these methods to be shared
the way we want.
In a nutshell, the problem is that each builder type returns itself, so that
calls can be chained. Setters inherited from an abstract-class/interface
return the type of that abstract interface, which only has a small subset of
the methods of the implementation. This really limits how calls can be chained.
The explanation above seems a little, well, abstract when I re-read it, so
maybe an example will clarify what I'm trying to say. Consider:
{code}
abstract class FooBasedBuilder {
public FooBasedBuilder withFoo(Foo foo) { ... return this;}
public abstract Bar build();
}
class BarBuilder extends FooBasedBuilder {
public BarBuilder withBoo(Boo boo) { ... return this; }
public Bar build() {...}
}
// WORKS!
new BarBuilder()
.withBoo(...) //returns BarBuilder
.withFoo(...) // BarBuilder has a withFoo() method, so this works.
.build()
// ERROR!
new BarBuilder()
.withFoo(...) // returns FooBuilder reference
.withBoo(...) // FooBuilder type doesn't know about withBar(), so this is
a compilation error!
{code}
As the examples above show, with the abstract-class-for-code-sharing design,
the order that setters are called in matters. Once a parent class method is
called, subclass methods can't be chained on.
Maybe there's a way around this, but I haven't been able to find one after
spending an hour or so racking my brains about it. With this in mind, if no
one can find an alternative, I'm going to go back to removing the code-sharing
portion of this patch, as much as that sucks.
> 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 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] [Commented] (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:comment-tabpanel=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 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] [Commented] (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:comment-tabpanel=15170676#comment-15170676
]
Jason Gerlowski commented on SOLR-8097:
---
Gonna revert my different name (updateLeadersOnly) as Anshum initially
suggested, and keep the name {{updatesToLeaders}} as it is for now. Maybe the
name/behavior/Javadocs should change, but I'd rather not touch that in this
JIRA, as I don't have a great understanding of the background there, and it'd
take some time to look into that.
If there's still changes regarding that param, I'm happy to do that in a
separate JIRA/patch if you guys want.
> 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 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] [Commented] (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:comment-tabpanel=15157463#comment-15157463 ] Anshum Gupta commented on SOLR-8097: Thanks Hoss. I'll work on adding support for this in SOLR-6312 without changing the default. > 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 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] [Commented] (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:comment-tabpanel=15157414#comment-15157414 ] Hoss Man commented on SOLR-8097: I'm guessing mark is remembering SOLR-6312. I don't have a strong opinion about the default behavior -- but as for "that param isn't even used right now and that seems strange" that's the heart of SOLR-6312, and that issue mentions reasons why supporting an explicit "false" to do round robin updates can be useful in some usecases. > 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 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] [Commented] (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:comment-tabpanel=15157403#comment-15157403 ] Mark Miller commented on SOLR-8097: --- Yup, looks like someone just made it default to true. I think [~hossman_luc...@fucit.org] or someone has mentioned valid reasons for keeping the option, so probably we should fix it rather than remove the param in 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: 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 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] [Commented] (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:comment-tabpanel=15157394#comment-15157394 ] Anshum Gupta commented on SOLR-8097: Ah ok. I was just looking at the code and it might just be an idea thing, but seems like that param isn't even used right now and that seems strange. but yes, I agree that is 'should' only prefer sending the request to the leader. I'll dig more into 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 > > > 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] [Commented] (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:comment-tabpanel=15157378#comment-15157378 ] Mark Miller commented on SOLR-8097: --- I don't think you are right. All it does it prefer leaders. They are tried first, and the replicas are tried after rather than full random. The javadoc is incorrect. > 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 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] [Commented] (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:comment-tabpanel=15157365#comment-15157365 ] Anshum Gupta commented on SOLR-8097: I think we should not use 'prefer' as it's not a preference but an explicit choice to send the request to the shard leaders. It wouldn't fall back to non-leaders if the preferred choice isn't available. > 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 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] [Commented] (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:comment-tabpanel=15157357#comment-15157357 ] Mark Miller commented on SOLR-8097: --- bq. updateLeadersOnly If you want to make it more specific, I believe it should be preferLeaders. updateLeadersOnly does not really make sense. > 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 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] [Commented] (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:comment-tabpanel=15157327#comment-15157327 ] Anshum Gupta commented on SOLR-8097: Thanks Jason. This looks good to me, how ever I'm debating about using an interface instead of an abstract class as that would do away with the single-inheritance restriction. I don't feel too strongly about it so I'm fine with either. Here are a few minor things I spotted so far: * updateLeadersOnly - let's just keep that as updatesToLeaders for consistency. It would make things easier for users and future developers. If you want to change it, it's best to change it throughout the file. * Unused imports cleanup * Javadocs wouldn't render correctly as the @return tag swallows the return char. A couple of changes in the patch seem unrelated. Let's move them into their own issue: * CUSC change seems unrelated. * BinaryResponseParser being used as default in the HttpSolrClient constructor and LBHttpSolrClient. 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. > 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] [Commented] (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:comment-tabpanel=15156283#comment-15156283 ] Anshum Gupta commented on SOLR-8097: I'll take a look tomorrow as I'd be away from the computer for most part of 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 >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] [Commented] (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:comment-tabpanel=15156251#comment-15156251 ] Jason Gerlowski commented on SOLR-8097: --- Just wanted to give this issue a little 'bump'. If there's not much interest in this issue, that's fine with me. Not trying to push this over other important things people are working on. Just wanted to make sure it wasn't missed on accident. So if you're interested in seeing this go forward and have time to take a look, please review the attached patch, and let me know what you'd like tweaked/improved in the next iteration. > 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] [Commented] (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:comment-tabpanel=15139189#comment-15139189 ] Anshum Gupta commented on SOLR-8097: Thanks for the patch Jason. The patch doesn't apply cleanly on master but I tried looking at the patch through the diff file directly and this is a good direction to move in. Let's cover other Clients too and I'd leave out Embedded for now and tackle that one in the end. > 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
[jira] [Commented] (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:comment-tabpanel=15137198#comment-15137198 ] Jason Gerlowski commented on SOLR-8097: --- Hi all, Just wanted to give this issue a little 'bump'. If there's not much interest in this issue, that's fine with me. Not trying to push this over other important things people are working on. Just wanted to make sure it wasn't missed on accident. So if you're interested in seeing this go forward and have time to take a look, please review the attached patch, and let me know what you'd like tweaked/improved in the next iteration. > 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
[jira] [Commented] (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:comment-tabpanel=15138253#comment-15138253
]
Jason Gerlowski commented on SOLR-8097:
---
Looks like the only remaining *Server variants are {{EmbeddedSolrServer}}
(appears to only be used in tests, and the map-reduce contrib module), and
{{EmbeddedTestSolrServer}} (lives in the morphline-core contrib module). My
guess is that it's probably not worth writing a builder for these, since their
use is restricted, and they don't really suffer from the too-many-ctor-args
problem that the other SolrClients seem to.
> 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
[jira] [Commented] (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:comment-tabpanel=15138268#comment-15138268 ] Shawn Heisey commented on SOLR-8097: We get semi-regular questions about EmbeddedSolrServer on the mailing lists and sometimes in the IRC channel. Users ARE using it. The reason I thought it could maybe use a builder pattern is that the way to create an instance is kind of arcane and non-obvious ... but I suppose that could be called a feature, because it might discourage people from using it. The embedded server is not recommended for general use, but it does make sense for certain highly specialized use cases. > 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
[jira] [Commented] (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:comment-tabpanel=15137480#comment-15137480 ] Shawn Heisey commented on SOLR-8097: I like the idea of going with a builder pattern. I would probably start with initial work on HttpSolrClient, work out the kinks there, then do the Concurrent and LB objects, and tackle Cloud last. I would do it this way so that each class can utilize methods from the "lower" class. I don't know if it makes sense to create a "SolrClientBuilder" interface or abstract class, but that can be investigated. If the embedded server needs similar treatment, then we should see whether it makes any sense to incorporate changes into the parent class -- SolrClient. A builder pattern for the embedded server probably does make sense, but I haven't looked deeper to verify. To delay the impact to client programs as long as possible, we should skip branch_5x, and just make the change in the master branch. The old constructors can be removed in master once branch_6x is created. > 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
[jira] [Commented] (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:comment-tabpanel=15138100#comment-15138100 ] Shawn Heisey commented on SOLR-8097: In master (6.0), most of the *Server variants should be removed, as they were deprecated in 5.0. EmbeddedSolrServer is the exception there. I haven't looked at the code in master yet to verify whether those classes are gone. > 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
[jira] [Commented] (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:comment-tabpanel=15137983#comment-15137983 ] Jason Gerlowski commented on SOLR-8097: --- Hey Shawn, thanks for the review. I hadn't realized this JIRA was intended to cover each of the SolrClient implementations. The list of CloudSearchClient ctors in the description narrowed my focus; on a re-read it's clear that was just an example. With that in mind all your feedback makes sense to me. I'll expand this patch out to the other SolrClients then, in a way that leverages inheritance where possible. Since I was focused more narrowly before, I'm also not sure what makes the most sense with regards to also covering SolrServer. I'll give your suggestions a shot and check back in with a patch or an update. > 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
[jira] [Commented] (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:comment-tabpanel=15091375#comment-15091375 ] Shawn Heisey commented on SOLR-8097: Code written and compiled with a particular version of SolrJ should work properly if the SolrJ jar is replaced with a newer minor version. This compatibility is not expected if the jar is replaced with a newer major version. The general goal is that the Solr Java API will not change in minor versions, but this is not guaranteed. The later discussion related to back-compat talks about some new annotations designed to make it more clear which APIs will be static and which are expected to change. The SolrJ API will always be guarded more closely against change than the rest of Solr, because it is the API that is used most commonly in user code. When public APIs *do* change in a minor version, the old API is generally marked as deprecated, and the API is completely removed from the next major version (trunk), so that existing code will still compile and function properly. > 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 > > 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] [Commented] (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:comment-tabpanel=15091257#comment-15091257 ] Jason Gerlowski commented on SOLR-8097: --- My comment above implies some questions about the measures we try to take in ensuring backward-compatibility in SolrJ code. I'm still new to the Solr community, so I'm not quite sure how things are typically done here. My assumption up to this point is that core SolrJ APIs (i.e. Java ctor/method signatures) can only be changed by a major release. (This appears to have been confirmed by most of the discussion on the recent "Breaking Java back-compat in Solr" mailing list thread, by my reading of it at least.) I'm also a little unclear on what "deprecation" means within the Solr community. Can a deprecation warning appear on a non-major release? How long does a deprecation warning typically stay around before the API can be removed altogether? Anyone have a pointer to an article/wiki where I can read up on this? That said, this isn't relevant/necessary to start working on a patch, as long as I don't touch any of the existing ctors. So that's how I'll go forward with things until I hear feedback otherwise. I'll start putting together a purely-additive patch off of trunk later tonight or tomorrow. > 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 > > 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] [Commented] (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:comment-tabpanel=15090826#comment-15090826 ] Jason Gerlowski commented on SOLR-8097: --- I'm gonna take a first pass at this. Is the goal/hope for this JIRA that the builder will allow us to deprecate/remove the CloudSearchClient ctors? (If so, should I remove them in this patch?) Or is the hope just that this will prevent additional ctors from being needed in the future? > 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 > > 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
